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

feat(Relay::Node) extend node & nodes fields using definition block #552

Merged
merged 4 commits into from
Feb 22, 2017

Conversation

xuorig
Copy link
Contributor

@xuorig xuorig commented Feb 17, 2017

As mentioned in #550 by @rmosolgo. It think its even nicer to allow anything to be redefined on the standard node and nodes field.

Copy link
Owner

@rmosolgo rmosolgo 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 to me! How about keeping the old test just as an end-to-end test, and to make sure that kwargs are also handled properly?

end

node_field.name = "nod3"
node_field.description = "The Relay Node Field"
Copy link
Owner

Choose a reason for hiding this comment

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

I noticed these assign the same values as assigned above, are these meant to be assertions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh. yes. I need ☕️

end

node_field.name = "nodez"
node_field.description = "The Relay Nodes Field"
Copy link
Owner

Choose a reason for hiding this comment

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

(same as above)

@xuorig
Copy link
Contributor Author

xuorig commented Feb 17, 2017

Added back the tests!

@rmosolgo
Copy link
Owner

Sweet :D

@rmosolgo rmosolgo merged commit 9783a3a into rmosolgo:master Feb 22, 2017
@xuorig xuorig deleted the extending-relay-node-fields branch February 22, 2017 03:10
@rmosolgo rmosolgo added this to the 1.5.0 milestone Mar 9, 2017
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