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

Propagate arguments of relay.NodeField to Field #1036

Merged
merged 5 commits into from
Sep 27, 2019

Conversation

tdiam
Copy link
Contributor

@tdiam tdiam commented Jul 16, 2019

Closes #1035

Question: Should all **kwargs from NodeField's constructor be propagated or should there be any check to ensure the field gets no arguments other than id?

Edit 1: I also moved the "The ID of the object" description to the ID argument instead of the whole Field, since it seemed more appropriate, and included description in the propagated arguments.

Verified

This commit was signed with the committer’s verified signature.
tdiam Theodore Diamantidis
@tdiam tdiam force-pushed the graphene-nodefield-issue branch from dac3d42 to 675743f Compare July 16, 2019 22:31

Verified

This commit was signed with the committer’s verified signature.
tdiam Theodore Diamantidis
@tdiam tdiam force-pushed the graphene-nodefield-issue branch from 17820fc to 8746230 Compare July 17, 2019 08:13

Verified

This commit was signed with the committer’s verified signature.
tdiam Theodore Diamantidis
Copy link
Member

@jkimbo jkimbo left a comment

Choose a reason for hiding this comment

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

Thanks @tdiam ! I've added a small suggestion but other than that this looks good.

Verified

This commit was signed with the committer’s verified signature.
tdiam Theodore Diamantidis
Copy link
Member

@jkimbo jkimbo left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@jkimbo jkimbo merged commit 7c7876d into graphql-python:master Sep 27, 2019
@tcleonard
Copy link
Collaborator

tcleonard commented Jan 26, 2021

Could we also merge this in the v2 branch? Some librairies (like graphene-django) are still relying on the v2...

I created a pull request to cherry pick this into the v2 branch: #1307

tcleonard pushed a commit to loft-orbital/graphene that referenced this pull request Jan 26, 2021
* Propagate name, deprecation_reason arguments of relay.NodeField to Field

* Allow custom description in Node.Field and move ID description to ID argument

* Add test for Node.Field with custom name

* Add tests for description, deprecation_reason arguments of NodeField

* Pass all kwargs from NodeField to Field
ekampf pushed a commit that referenced this pull request Apr 23, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* Propagate name, deprecation_reason arguments of relay.NodeField to Field

* Allow custom description in Node.Field and move ID description to ID argument

* Add test for Node.Field with custom name

* Add tests for description, deprecation_reason arguments of NodeField

* Pass all kwargs from NodeField to Field

Co-authored-by: Theodore Diamantidis <diamaltho@gmail.com>
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.

relay.NodeField does not propagate arguments to Field
5 participants