-
Notifications
You must be signed in to change notification settings - Fork 112
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
⚡️ Accept array of string as valid input schema path. #134
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this look good to me!
The only thing that's needed before landing is to update docs/CHANGELOG.md, to add this new feature under ### New features:
. There's also probably some documentation elsewhere in docs/ that needs to be updated to document the new yaml file format possibility.
generate/config.go
Outdated
var schemaRaw []byte | ||
schemaRaw, err = ioutil.ReadFile(filename) | ||
if err != nil { | ||
return nil, fmt.Errorf("unable to open schema: %w", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think genqlient has its own errorf
wrapper -- is it appropriate to use that here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it appropriate to use that here?
You are right. Whole code should have one unified style. Pushed f471c95.
How does this sound to you?
It would be more specific. WDYT? |
That looks great! It makes clear you can use glob patterns, too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thanks so much.
I don't know if I actually have the ability to merge this in, but hopefully someone else on this review does if I don't.
Hello @hsblhsn! Thanks for the contribution. Could you sign the Khan Academy Contributor License Agreement when you have a moment? |
Multiple schema-files are now supported as of #134, but the support was a bit different from how we did multiple operation-files. Before anyone starts to depend on the ways the syntaxes differ, let's just make them the same. Since it's easy, I also added support for having just a single operations-file. Test plan: make check
…137) Multiple schema-files are now supported as of #134, but the support was a bit different from how we did multiple operation-files. Before anyone starts to depend on the ways the syntaxes differ, let's just make them the same. Since it's easy, I also added support for having just a single operations-file. I also realized while writing this that the type-change is technically breaking (if you call from Go), so documented it as such. I think this is unlikely to affect many people. Test plan: make check
This PR adds a feature that accepts both string and array of string as valid input in
schema
key in config.This feature is completely backward compatible. It does not deprecate or break the previous config file format.
Most of the code is sourced from https://github.com/99designs/gqlgen.
Closes #88