Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PLEASE REVIEW: Add error return value to resolvers (fixes for recent graphql-go change) #10

Closed
wants to merge 1 commit into from

Conversation

augustoroman
Copy link

This corrects the function signatures for all graphql.ResolverFn handlers
so that the graphql-go/handler package pull-request can pass tests.

However, I'm not too familiar with this code and I'm not confident that my
changes are semantically correct -- some of the handlers look like they
may want to return errors rather than returning nil. In a few cases
(e.g. plural.go), I went ahead and added some error returns, but these
really should be audited and tested.

…ge).

This corrects the function signatures for all graphql.ResolverFn handlers
so that the graphql-go/handler package pull-request can pass tests.

However, I'm not too familiar with this code and I'm not confident that my
changes are semantically correct -- some of the handlers look like they
may want to return errors rather than returning nil.  In a few cases
(e.g. plural.go), I went ahead and added some error returns, but these
really should be audited and tested.
@sogko
Copy link
Member

sogko commented Dec 14, 2015

Hi @augustoroman

Thanks for this PR contribution! I'll close this since its similar to #11

Regarding the appropriate error messages, we might go over it in a separate PR.

Thanks again!
Cheers!

@sogko sogko closed this Dec 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants