Skip to content

Conversation

clemens-tolboom
Copy link
Contributor

…both set on the same type.

Not sure what to choose I removed both as these do not help for the example imho.

See

raise ValueError("The options 'only_fields' and 'exclude_fields' cannot be both set on the same type.")

@coveralls
Copy link

coveralls commented Oct 17, 2019

Coverage Status

Coverage remained the same at 97.343% when pulling 033f76f on clemens-tolboom:patch-2 into 0544f81 on graphql-python:master.

@jnak
Copy link
Collaborator

jnak commented Oct 23, 2019

Hello, thanks for catching this and sending a PR. I feel it's important that only_fields and exclude_fields are documented in the README. How about we keep the exclude_fields declaration since it tends to be the most common and adding a comment saying that 'only_fields` can be used to whitelist fields?

@clemens-tolboom
Copy link
Contributor Author

I've rephrased the PR. Hope that's better :)

Copy link
Collaborator

@jnak jnak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@jnak jnak merged commit 89c3726 into graphql-python:master Oct 23, 2019
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.

3 participants