Optimization Hints

[FrownyFace]

usage (inside graphql_schema/clinical_notes.py

@resolver_hints(select_related=["locked_by"])
def resolve_lock(self, _info):

• black formatting

[cassiemeharry]

Since you're apparently applying Black to the project, could you add/update the pyproject.toml file so the files stay formatted?

[cassiemeharry]

These should be set objects.

self.prefetch_related = set(prefetch_related) if prefetch_related else set()
self.select_related = set(select_related) if select_related else set()
self.only = set(only) if only else set()

[cassiemeharry]

Using a list comprehension only for its side effects is considered bad style. I see that it's doing this a few lines up as well, but that should be fixed.

If you work with set objects, you can skip the explicit loop entirely and express the intent of merging in the new entries directly:

select_related = sorted(set(select_related) | optimization_hints.select_related)

The sorted() call is only necessary because the rest of the library doesn't pass those fields around as sets like it should.

[cassiemeharry]

Same for prefetch_related here as well.

[AlecRosenbaum]

I haven't actually looked at the code here yet, but I think on forked repos we should probably follow the model of having master always stay up to date with upstream's master, then use a custom drchrono branch or something like that for changes that we've made to our fork.

Then this PR would probably be directed into a drchrono branch

Once changes for nick's feedback are in, I'll take a look at the code as well

[cassiemeharry]

You know, that's actually a good point. To keep this relatively up to date, we should avoid doing extra formatting changes if we can avoid it. Otherwise, we're going to forever run into merge conflicts when we merge the upstream version into our own.

@FrownyFace Can you please undo the Black formatting, and pare this PR down to the core changes?

[AlecRosenbaum]

@nickmeharry I'm pretty sure the project already uses black, I think they just do it with different line length settings. I think we just have to make sure black is actually respecting this project's settings

[cassiemeharry]

Ok. My point stands though, this diff is too big to maintain in a fork if we want any hope of keeping things up to date. I guess figure out the line length this project uses and format it that way, so only meaningful changes are present here?

[AlecRosenbaum]

@nickmeharry I agree with you. This project's pyproject.toml specifies 88, I think the formatter just didn't pick up the right pyproject file

[FrownyFace]

un-formatted + nick changes incorporated @AlecRosenbaum

[AlecRosenbaum]

Who is taking over ownership of this change/project?

@VKuber Any thoughts on who will be taking over DW-11248?

[MicGong]

This might be resolved with graphene-django 2.4.0 release.

[AlecRosenbaum]

Could you remove my review request from this PR? I don't have edit access anymore and this has been sitting in my review requests for quite some time now.

I'm not sure who has access to do this still, maybe @cassiemeharry?