Modify documentation to be more clear about resolvers being static

[allardhoeve]

See #951

[jkimbo]

@allardhoeve decorating resolvers with @staticmethod is not necessary but thanks for the change from self

[allardhoeve]

@allardhoeve decorating resolvers with @staticmethod is not necessary but thanks for the change from self

Good suggestions, I'll apply them next week and then re-request the merge.

I disagree about the necessity of @staticmethod though. I understand that you unbind the method in the resolver code, so technically for GraphQL it is not needed. But there are other things to consider besides this.

1. IDEs such as PyCharm will warn that the method, because it is still a method even though GraphQL unbinds it, should be marked as such.
2. These IDEs will also warn you that coding standards indicate that the first argument to a method is normally self.

Of course you can ignore these standards, but they are confusing to code reviewers and to programmers new to GraphQL, such as myself.

Lastly the methods are no longer callable from the outside of GraphQL without also using the unbind magic, because the signature no longer matches a method, because the self argument is not in the signature. This would prevent you from calling the resolver properly, for example in unittests.

I feel that the documentation for GraphQL should be as clear as possible and should reflect this rather odd unbinding of the resolver well, so new programmers are sure about the interface.

You can of course always leave the decorator off you own functions if you want. To me this is a documentation issue.

[ProjectCheshire]

I disagree on needing static also because resolvers do not need to be defined next to the fields, but may be defined outside of, and may very well be for more complex ones.

In that case, adding staticmethod makes it confusing. And doing it only for resolvers when defined with the field makes it inconsistent and confusing.

[allardhoeve]

Indeed, not every resolver needs to be a staticmethod. If you use an outside function as a resolver, then no, you definitely shouldn't use a decorator. But as defined in a class, they probably should be marked as such, as they are essentially functions after becoming unbound by GraphQL.

My request is not at all to force everyone to use static method decorators everywhere, but just that we improve the documentation on resolvers better, because they are inconsistently documented currently.

My suggestion is still that for resolvers that are defined on NodeType objects, we still document them as static methods, as they are. And the part in the documentation on resolvers is easily missed.

But the situation improves with changing self with the subject of the function such as ship, thanks!

[phalt]

Is it possible to add a documentation note about this issue with some IDEs and leave the default examples without? Either way, we should progress this PR or close it.