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

Remove default values from initializers #1461

Merged

Conversation

namolnad
Copy link
Contributor

Improves clarity and prevents mistakes showcased in #1460 by removing default values for initializers

@apollo-cla
Copy link

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

Copy link
Contributor

@designatednerd designatednerd left a comment

Choose a reason for hiding this comment

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

I think this is generally a good start - I want to do some more rethinking before an official release is cut, since I think there's still some pretty significant opportunities for developers to accidentally have different stores.

May wait a day or two to merge this until I have that bit ready to go, but overall, this is a great start 👍

@designatednerd
Copy link
Contributor

I poked at what my idea was and it was WAY over-complex. This is much more straightfoward, so let's go with this approach. I'll update some docs post-merge.

@designatednerd designatednerd merged commit 2a1d35a into apollographql:main Oct 19, 2020
@designatednerd designatednerd added this to the Next Release milestone Oct 19, 2020
@namolnad
Copy link
Contributor Author

Sounds great — glad it was helpful. Thanks for looking at it!

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.

3 participants