-
Notifications
You must be signed in to change notification settings - Fork 9
Highlight that the name is a recommendation #35
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
Conversation
andimarek
left a comment
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.
Looks good, but I would apply the changes suggested by @glen-84
Co-authored-by: Glen <glen.84@gmail.com>
Co-authored-by: Glen <glen.84@gmail.com>
Co-authored-by: Glen <glen.84@gmail.com>
|
Thanks for the review @glen-84! |
glen-84
left a comment
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.
If the templates are changed, then all existing specs should be updated to match.
I would push back from that. Changing the existing specs without the authors approval sounds not nice. And contacting them and getting their approval is a lot of effort (well, maybe not today because it's only @andimarek contributing but in the future, that's not something I'm really keen on doing) |
|
Well ...
|
I think I would. For an example, Older specs will become inconsistent but I think this is fine. GraphQL has overindexed on quality and this has driven a number of contributors out. I'm not saying we should break everything but some chaos in this repo is perfectly fine by me as long as it allows us to move faster. |
You would contact Andi for permission to change This isn't Geocities, these are formal standards referenced by many (we hope).
This will cause fragmentation over time, at which point the templates become less useful. But anyway, it's just a suggestion. 🙂 |
I would :). It's not about the magnitude of the change, it's about setting clear ownership. My desk is typically a mess but it's "my" mess. I'd be mad if someone cleans it up!
If you're ok with that specific change, do you mind changing your review to "comment" or "approve" so that we can move forward? Given I have @andimarek approval I think we can merge this if you don't have a strong objection. Feel free to open a follow up PR to keep the existing scalars in check. |
A specification mentions a "recommended" name but users are free to use other values. The recommended name also doesn't have to match the url path segment.
Follow up from #34