-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Update validation page. #1803
base: source
Are you sure you want to change the base?
Update validation page. #1803
Conversation
@mandiwise is attempting to deploy a commit to the The GraphQL Foundation Team on Vercel. A member of the Team first needs to authorize it. |
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.
This is looking good! Here are some light editorial suggestions (many of which apply to the original content).
|
||
```graphql | ||
# { "graphiql": true } | ||
# INVALID: favoriteSpaceship does not exist on Character | ||
{ | ||
query { |
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.
Interested to know why you're moving away from the query shorthand here? It tends to be very rare that I see this - either the operation has a name and/or variables in which case you need query
, or it has neither in which case it's very rare to see it in my experience.
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've tended to see the opposite (inclusion of the query
keyword) so I included it for clarity after all of the keywords are introduced on the operation pages. But I can remove it throughout if the shorthand syntax is the preferred convention.
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.
Let's worry about that afterwards, if at all; it's much easier to replace /^query {$/
with {
than the reverse - so keep going with the query
keyword for now.
Interesting that your experience differs; I wonder what the ecosystem as a whole sees!
Co-authored-by: Benjie <[email protected]>
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 to me! 🙌
Description
This PR contains updated validation content for the Learn docs. Key changes include:
@benjie @jorydotcom