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.

Sign up for GitHub

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

Jump to bottom

DSL: Allow to use GraphQL keys named type #151

Merged
merged 1 commit into from
Oct 14, 2020
Merged

DSL: Allow to use GraphQL keys named type #151

merged 1 commit into from
Oct 14, 2020

Conversation

gdalmau
Copy link
Contributor

@gdalmau gdalmau commented Oct 13, 2020

Fixes #150

This is a PR trying to fix #150

@KingDarBoja
Copy link
Contributor

Nice catch, in fact, graphql-core does use type_ for internals ;)

@leszekhanusz leszekhanusz marked this pull request as ready for review October 13, 2020 16:48
@leszekhanusz leszekhanusz changed the title Allow to use GraphQL keys named type DSL: Allow to use GraphQL keys named type Oct 13, 2020
@gdalmau
Copy link
Contributor Author

gdalmau commented Oct 13, 2020

Nice catch, in fact, graphql-core does use type_ for internals ;)

@KingDarBoja should I rename it to type_?

@leszekhanusz
Copy link
Collaborator

This is a variable only used in the class no ? So it should be fine as a private variable.
It could stay as _type for me.

KingDarBoja
KingDarBoja approved these changes Oct 13, 2020
View reviewed changes
Copy link
Contributor

@KingDarBoja KingDarBoja left a comment

Choose a reason for hiding this comment

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

No need to rename, just making a point of the usage of prefix and suffix for internals, everything looks great to me 🚀

@gdalmau
Copy link
Contributor Author

gdalmau commented Oct 13, 2020

I tried the example in the issue linked #150 and it works with this version, so it's ready to merge 😄

@KingDarBoja KingDarBoja added the type: bug An issue or pull request relating to a bug label Oct 13, 2020
@leszekhanusz leszekhanusz merged commit 317ad4b into graphql-python:master Oct 14, 2020
@gdalmau gdalmau deleted the fix_key_named_type branch December 20, 2020 13:14
@leszekhanusz leszekhanusz mentioned this pull request Dec 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leszekhanusz leszekhanusz leszekhanusz approved these changes

@KingDarBoja KingDarBoja KingDarBoja approved these changes

Assignees
No one assigned
Labels
type: bug An issue or pull request relating to a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DSL not allowing to use a key named "type"
3 participants
@gdalmau @KingDarBoja @leszekhanusz