consistency and style fix#811
Conversation
Mashimiao
force-pushed
the
style-consistent-fix
branch
from
a36389c to
265c768
Compare
config.md
Outdated
config.md
Outdated
There was a problem hiding this comment.
capabilities is already using four-space indents (in the list below). Can we use them here while we're touching the lines?
config.md
Outdated
There was a problem hiding this comment.
Also want deeper indents here to make it clearly part of the hard list entry.
config.md
Outdated
There was a problem hiding this comment.
I don't think readers are likely to forget which property we're talking about ;). Can we shorten this line? Maybe:
Values should be, and runtimes SHOULD understand, valid [`GOOS`][go-environment] values.
config.md
Outdated
There was a problem hiding this comment.
Can we just use “Runtimes”?
$ git describe --always
v1.0.0-rc5-148-g45c3fd4
$ git grep -i runtime.implementations *.md | wc -l
2
$ git grep -i runtimes *.md | wc -l
19
265c768 to
f035b58
Compare
|
all comments have been updated. PTAL |
config.md
Outdated
There was a problem hiding this comment.
When I asked for four-space indents here, I meant all the lines after * **capabilities** … (sorry for not clarifying). So it should be:
* **`capabilities`** (object, OPTIONAL) is an object containing arrays that specifies the sets of capabilities for the process(es) inside the container.
Valid values are platform-specific.
For example, valid values for Linux are defined in the [capabilities(7)][capabilities.7] man page, such as `CAP_CHOWN`.
Any value which cannot be mapped to a relevant kernel interface MUST cause an error.
`capabilities` contains the following properties:
There was a problem hiding this comment.
updated, but there are a lot of sentences do not start with four-space indents, will not fix them in this PR.
I can submit a follow-PR to format all of them.
Mashimiao
force-pushed
the
style-consistent-fix
branch
from
f035b58 to
7858d03
Compare
config.md
Outdated
There was a problem hiding this comment.
Fwiw, I think "Runtime implementations" is better than 'Runtimes" and I'd change the three 'Runtimes' below to that too. My $.02, your call to make.
There was a problem hiding this comment.
... I'd change the three 'Runtimes' below to that too.
Would you update the other instances (beyond those already touched by this PR) too?
Can you put a finger on why you likw including "implementations" better? A bare "runtimes" is shorter, and I can't think of a non-implementation runtime.
config.md
Outdated
There was a problem hiding this comment.
Suggest 'reading or processing' here and below. I'm not a big fan of '/' over 'or'. Again, my $.02, your call.
Mashimiao
force-pushed
the
style-consistent-fix
branch
4 times, most recently
from
204bb77 to
9bc1ef8
Compare
|
rebased. @opencontainers/runtime-spec-maintainers PTAL |
replace file sytem with filesystem for consistent in context keep one sentence per line Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
Mashimiao
force-pushed
the
style-consistent-fix
branch
from
9bc1ef8 to
b92cf90
Compare
|
rebased around #846 |
|
LGTM |
1 similar comment
|
LGTM |
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>
5 participants
replace file sytem with filesystem for consistent in context
keep one sentence per line
Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com