Add links to contributed custom scalar specs at scalars.graphql.org#1009
Add links to contributed custom scalar specs at scalars.graphql.org#1009leebyron merged 7 commits intographql:mainfrom
Conversation
✅ Deploy Preview for graphql-spec-draft ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Feel free to comment on the wording |
|
I am not 100% regarding the process here: I don't see this as the normal trivial editorial change (e.g. typo), but it is also no real change in the spec. Therefore I am leaning to treat it as editorial change. Keen to hear what others think @benjie @michaelstaib @leebyron |
benjie
left a comment
benjie
left a comment
There was a problem hiding this comment.
Suggest we shrink this PR somewhat and ensure the changes are in non-normative notes. I've included some suggestions, but you might not want to use them verbatim - edit as you see fit.
Co-authored-by: Benjie <[email protected]>
Co-authored-by: Benjie <[email protected]>
Co-authored-by: Benjie <[email protected]>
|
Thanks @benjie ! I've put through all your comments |
|
|
||
| ```graphql example | ||
| scalar UUID @specifiedBy(url: "https://tools.ietf.org/html/rfc4122") | ||
| scalar URL @specifiedBy(url: "https://tools.ietf.org/html/rfc3986") |
There was a problem hiding this comment.
I honestly think these are not good examples: we should not encourage people to link directly
to RFCs. I would even suggest to remove the UUID example. A RFC is great to be the basis of a custom scalar, but it is almost never sufficient to be a good guide. We should people actively encourage to write dedicated custom scalars specs.
There was a problem hiding this comment.
Agreed; writing a scalars.graphql.org spec that calls out to the relevant RFC seems wise. I'm not sure if that needs to be done in this PR though?
There was a problem hiding this comment.
It definitely doesn't need to be addressed in this PR; it's already referenced on line 407, this is just a copy. Can someone file it for follow-up though?
benjie
left a comment
benjie
left a comment
There was a problem hiding this comment.
Looking good, let's trim those final two lines and then I think we're good to go (after you run npm install && npm run format to appease Prettier) 👍
Co-authored-by: Benjie <[email protected]>
|
I have spoken the magic words I'm ready for a final review @benjie |
|
Hi @benjie could you please have another review? |
mjmahone
left a comment
mjmahone
left a comment
There was a problem hiding this comment.
These changes seem clean to me, all in favor of shipping!
leebyron
added
the
✏️ Editorial
|
Nice work! Glad to have these references in place |
5 participants
The PR follows on from the launch of scalars.graphql.org.
https://github.com/graphql/graphql-scalars
https://graphql.org/blog/2023-01-14-graphql-scalars/
As discussed in the January APAC working group meeting, this PR adds how to contribute custom scalar specs, links to spec templates, and adds examples.
cc @andimarek