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

Stop default flag values from overriding custom config #511

Merged

Conversation

klujanrosas
Copy link
Contributor

  • Check if a custom config for queries hasn't been setup, else fallback to flags.queries

@apollo-cla
Copy link

@klujanrosas: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@klujanrosas
Copy link
Contributor Author

klujanrosas commented Jul 25, 2018

I checked Travis and it tells me that $STAGING_API_KEY is missing

@shadaj
Copy link
Contributor

shadaj commented Jul 25, 2018

Hi @klujanrosas! Allowing the queries flag to override the config was actually an intended behavior, as users may sometimes want to specifically generate types for a subset of queries.

However, we didn't catch that the queries would always be overriden by the default value. Maybe this could be fixed by only allowing overrides when the flag is not the default value?

Also, we're aware of the Travis issue and will hopefully have a fix for it later today.

@shadaj shadaj added 🐞 bug and removed bugfix labels Jul 25, 2018
@klujanrosas
Copy link
Contributor Author

Hi @shadaj , I went ahead and implemented your proposed solution.
Let me know if there are improvements or changes to be made.

Thanks!

@shadaj
Copy link
Contributor

shadaj commented Jul 27, 2018

@klujanrosas could you update with the latest master? We have another fix for the failing Travis issue that should make your changes pass.

@shadaj
Copy link
Contributor

shadaj commented Jul 30, 2018

Thanks so much for the fix @klujanrosas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants