Skip to content

Derive Default for schema ast types #16

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

Merged
merged 2 commits into from
Jul 24, 2018

Conversation

leoyvens
Copy link
Contributor

This makes them easier to construct.

This makes them easier to construct.
@tailhook
Copy link
Collaborator

While it gives the shortest code, is using Default a best way to construct these values? Especially considering that some of them are invalid when constructing default (i.e. empty name).

Maybe it's better to make a fn new(name) method? (except for Schema and Document)

@leoyvens
Copy link
Contributor Author

For my use case which are unit tests empty names work fine, the default value is not completely unusable.

@tailhook
Copy link
Collaborator

It would make more sense if trait would be TestData::generate() instead of Default::default(). Would new("test_data") be too cumbersome for you?

@leoyvens
Copy link
Contributor Author

That would be fine, I'll update this to add a new(name) instead of Default where that is better.

@leoyvens
Copy link
Contributor Author

Done, let me know what you think.

@tailhook tailhook merged commit 0543ca5 into graphql-rust:master Jul 24, 2018
@tailhook
Copy link
Collaborator

Looks fine. Merged. Thanks!

@tailhook
Copy link
Collaborator

Released as v0.2.2

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.

2 participants