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

schema: Drop pointers and nulls #662

Merged
merged 1 commit into from
Feb 6, 2017

Conversation

wking
Copy link
Contributor

@wking wking commented Jan 19, 2017

Maintainers feel (and I agree) that there's no point in explicitly allowing a null value when callers can simply leave the property unset. This commit removes all references to “pointer” and “null” from the JSON Schema to support that decision. While optional properties may sometimes be represented as pointer types in Go, optional properties should be represented in JSON Schema by not including the properties in the required array.

Spun off from this comment as requested by @RobDolinMS.

@Mashimiao
Copy link

This is part duplicated with #545

@wking
Copy link
Contributor Author

wking commented Jan 23, 2017

This is part duplicated with #545

Ah, yup. Happy to rebase if that lands first.

Maintainers feel (and I agree) that there's no point in explicitly
allowing a null value when callers can simply leave the property unset
[1].  This commit removes all references to "pointer" and "null" from
the JSON Schema to support that decision.  While optional properties
may sometimes be represented as pointer types in Go [2], optional
properties should be represented in JSON Schema by not including the
properties in the 'required' array.

[1]: opencontainers#555 (comment)
[2]: style.md "Optional settings should not have pointer Go types"

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking
Copy link
Contributor Author

wking commented Jan 23, 2017 via email

@mikebrow
Copy link
Member

mikebrow commented Feb 2, 2017

Reviewed looks good.

@crosbymichael
Copy link
Member

crosbymichael commented Feb 2, 2017

LGTM

Approved with PullApprove

1 similar comment
@hqhq
Copy link
Contributor

hqhq commented Feb 6, 2017

LGTM

Approved with PullApprove

@hqhq hqhq merged commit ce0783a into opencontainers:master Feb 6, 2017
@wking wking deleted the json-schema-no-pointer branch February 8, 2017 19:52
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request May 11, 2017
Catch the Markdown spec up with the JSON Schema change in 0927437
(schema: Drop pointers and nulls, 2017-01-18, opencontainers#662).  The Markdown is
canonical, so we could restore the explicit-null handling to the JSON
Schema instead, but the maintainers feel (and I agree) that there's no
point in explicitly allowing a null value when callers can simply
leave the property unset [1].

[1]: opencontainers#555 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request May 11, 2017
Catch the Markdown spec up with the JSON Schema change in 0927437
(schema: Drop pointers and nulls, 2017-01-18, opencontainers#662).  The Markdown is
canonical, so we could restore the explicit-null handling to the JSON
Schema instead, but the maintainers feel (and I agree) that there's no
point in explicitly allowing a null value when callers can simply
leave the property unset [1].

[1]: opencontainers#555 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
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.

5 participants