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

Clarification of non-null arguments wording #288

Closed
gtod opened this issue Mar 16, 2017 · 3 comments
Closed

Clarification of non-null arguments wording #288

gtod opened this issue Mar 16, 2017 · 3 comments

Comments

@gtod
Copy link
Contributor

gtod commented Mar 16, 2017

The spec currently has (my italics)

5.3.3.2 Required Non-Null Arguments

Explanatory Text

Arguments can be required. Arguments are required if the type of the argument is non‐null. If it is not non‐null, the argument is optional. When an argument type is non‐null, and is required, the explicit value null may also not be provided.

I think that italicised phrase should be and thus required. In fact, it should probably just be omitted. Or even:

Arguments can be required. If the argument type is non null the argument is required and furthermore the explicit value null may not be provided. If the argument type is not non‐null, the argument is optional.

Either that or I have misunderstood...

@robzhu
Copy link
Contributor

robzhu commented May 1, 2017

Thanks for spotting this. Agreed that this wording could be more clear. Please go ahead and submit a PR.

@wincent
Copy link
Contributor

wincent commented May 4, 2017

Arguments can be required. If the argument type is non null the argument is required and furthermore the explicit value null may not be provided. If the argument type is not non‐null, the argument is optional.

Thanks @gtod. This is a great improvement but the double negative in italics is a bit hard to parse. I think this would read better if we just replaced the italicized part with "Otherwise"; eg:

Arguments can be required. If the argument type is non-null the argument is required and furthermore the explicit value null may not be provided. Otherwise, the argument is optional.

gtod added a commit to gtod/graphql that referenced this issue May 4, 2017
wincent added a commit that referenced this issue May 4, 2017
Clairfy required non null arguments wording (#288)
@wincent
Copy link
Contributor

wincent commented May 4, 2017

Closed via #306

Thanks for the report, and for the fix @gtod.

@wincent wincent closed this as completed May 4, 2017
IvanGoncharov pushed a commit to IvanGoncharov/graphql that referenced this issue Jun 17, 2017
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

No branches or pull requests

3 participants