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

config: Collapse extensibility to a single MUST #916

Merged
merged 1 commit into from
Dec 18, 2019

Conversation

wking
Copy link
Contributor

@wking wking commented Aug 31, 2017

Generating an error seems like one potential violation of the requirement to ignore unknown properties. Compliance testing for the ignore requirement can cite the MUST I've written here for any noticeable runtime activity around the unknown property without needing a error-specific MUST.

We've had the two MUSTs since #510, citing opencontainers/image-spec#164. I'd asked for consolidated phrasing then, but hadn't followed up after the commit landed.

I've left a line mentioning the error activity as non-normative clarification, but am also happy to drop that line completely.

Also:

  • Update the unknown annotation entry to reference the generic extensibility section, because there's nothing annotation-specific in how we want runtimes to handle unknown keys.

  • Remove “reading or processing” language. This initially landed in Add text about extensions #510 with a bump in consistency and style fix #811. Some thought was put into this phrasing there and earlier in Add text about extensions #510, but we never got around to dropping this qualifier. However, the purpose of this qualifier is unclear to me. What is the point of compliance requirements for runtimes which don't read or process a configuration?

Generating an error seems like one potential violation of the
requirement to ignore unknown properties.  Compliance testing for the
ignore requirement can cite the MUST I've written here for any
noticeable runtime activity around the unknown property without
needing a error-specific MUST.

We've had the two MUSTs since 27a05de (Add text about extensions,
2016-06-26, opencontainers#510), citing [1].  I'd asked for consolidated phrasing
then [2,3], but hadn't followed up after the commit landed.

I've left a line mentioning the error activity as non-normative
clarification, but am also happy to drop that line completely.

Also:

* Update the unknown annotation entry to reference the generic
  extensibility section, because there's nothing annotation-specific
  in how we want runtimes to handle unknown keys.

* Remove "reading or processing" language.  This initially landed in
  27a05de with a bump in b92cf90 (consistency and style fix,
  2017-05-12, opencontainers#811).  Some thought was put into this phrasing there
  [4,5] and earlier in opencontainers#510 [6], but we never got around to dropping
  this qualifier.  However, the purpose of this qualifier is unclear
  to me.  What is the point of compliance requirements for runtimes
  which don't read or process a configuration?

[1]: opencontainers/image-spec#164
[2]: opencontainers#510 (comment)
[3]: opencontainers#510 (comment)
[4]: opencontainers#811 (comment)
[5]: opencontainers#811 (comment)
[6]: opencontainers#510 (comment)

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

wking commented Aug 31, 2017

Despite touching some MUST language, I consider this a patch-level change because the semantics are unchanged. Everything that was legal before is still legal, and everything that was illegal before is still illegal.

@vbatts
Copy link
Member

vbatts commented Dec 17, 2019

house keeping: this clarification does not break any versioning as the sentence with MUST is just inverted and made more clear.

LGTM

Approved with PullApprove

@tianon
Copy link
Member

tianon commented Dec 18, 2019

LGTM

Approved with PullApprove

@tianon tianon merged commit 903de9f into opencontainers:master Dec 18, 2019
@vbatts vbatts mentioned this pull request Jan 9, 2020
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.

3 participants