consistency and style fix #811

Mashimiao · 2017-05-12T12:45:25Z

replace file sytem with filesystem for consistent in context
keep one sentence per line

Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com

wking · 2017-05-12T18:11:36Z

config.md

  * Solaris: corresponds to "type" of the fs resource in [zonecfg(1M)][zonecfg.1m].
 * **`source`** (string, OPTIONAL) A device name, but can also be a directory name or a dummy.
  * Windows: the volume name that is the target of the mount point, \\?\Volume\{GUID}\ (on Windows source is called target).
  * Solaris: corresponds to "special" of the fs resource in [zonecfg(1M)][zonecfg.1m].
 * **`options`** (list of strings, OPTIONAL) Mount options of the filesystem to be used.
-  * Linux: supported options are listed in the [mount(8)][mount.8] man page. Note both [filesystem-independent][mount.8-filesystem-independent] and [filesystem-specific][mount.8-filesystem-specific] options are listed.
+  * Linux: supported options are listed in the [mount(8)][mount.8] man page.
+  Note both [filesystem-independent][mount.8-filesystem-independent] and [filesystem-specific][mount.8-filesystem-specific] options are listed.


I think you want deeper indenting here to show that this is part of the * Linux: … entry. For an example with a two-space indent, see here, although that's getting cleaned up to use four-space indents (which Pandoc likes better) in #724.

wking · 2017-05-12T18:12:26Z

config.md

+  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:


capabilities is already using four-space indents (in the list below). Can we use them here while we're touching the lines?

wking · 2017-05-12T18:13:07Z

config.md

@@ -144,7 +148,8 @@ For Windows, see [mountvol][mountvol] and [SetVolumeMountPoint][set-volume-mount

    * **`type`** (string, REQUIRED) - the platform resource being limited, for example on Linux as defined in the [setrlimit(2)][setrlimit.2] man page.
    * **`soft`** (uint64, REQUIRED) - the value of the limit enforced for the corresponding resource.
-    * **`hard`** (uint64, REQUIRED) - the ceiling for the soft limit that could be set by an unprivileged process. Only a privileged process (e.g. under Linux: one with the CAP_SYS_RESOURCE capability) can raise a hard limit.
+    * **`hard`** (uint64, REQUIRED) - the ceiling for the soft limit that could be set by an unprivileged process.
+    Only a privileged process (e.g. under Linux: one with the CAP_SYS_RESOURCE capability) can raise a hard limit.


Also want deeper indents here to make it clearly part of the hard list entry.

wking · 2017-05-12T18:16:33Z

config.md

  The runtime MUST generate an error if it does not support the specified **`os`**.
-  Bundles SHOULD use, and runtimes SHOULD understand, **`os`** entries listed in the Go Language document for [`GOOS`][go-environment].
+  Value for **`os`** SHOULD use, and runtimes SHOULD understand, **`os`** entries listed in the Go Language document for [`GOOS`][go-environment].


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.

wking · 2017-05-12T18:22:20Z

config.md

@@ -428,7 +433,7 @@ Keys MUST be unique within this map.
 Keys MUST NOT be an empty string.
 Keys SHOULD be named using a reverse domain notation - e.g. `com.example.myKey`.
 Keys using the `org.opencontainers` namespace are reserved and MUST NOT be used by subsequent specifications.
-Implementations that are reading/processing this configuration file MUST NOT generate an error if they encounter an unknown annotation key.
+Runtime implementations that are reading/processing this configuration file MUST NOT generate an error if they encounter an unknown annotation key.


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

Mashimiao · 2017-05-13T01:06:56Z

all comments have been updated. PTAL

wking · 2017-05-13T05:50:15Z

config.md

+* **`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.


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:

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.

TomSweeneyRedHat · 2017-05-15T11:17:34Z

config.md

@@ -323,7 +328,7 @@ For Windows based systems the user structure has the following fields:
 ## <a name="configPlatformSpecificConfiguration" />Platform-specific configuration

 [**`platform.os`**](#platform) is used to specify platform-specific configuration.
-Runtime implementations MAY support any valid values for platform-specific fields as part of this configuration.
+Runtimes MAY support any valid values for platform-specific fields as part of this configuration.


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.

... 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.

TomSweeneyRedHat · 2017-05-15T11:18:49Z

config.md

@@ -428,7 +433,7 @@ Keys MUST be unique within this map.
 Keys MUST NOT be an empty string.
 Keys SHOULD be named using a reverse domain notation - e.g. `com.example.myKey`.
 Keys using the `org.opencontainers` namespace are reserved and MUST NOT be used by subsequent specifications.
-Implementations that are reading/processing this configuration file MUST NOT generate an error if they encounter an unknown annotation key.
+Runtimes MUST NOT generate an error if they encounter an unknown annotation key when reading/processing this configuration file.


Suggest 'reading or processing' here and below. I'm not a big fan of '/' over 'or'. Again, my $.02, your call.

Mashimiao · 2017-05-23T07:52:23Z

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 · 2017-06-01T16:31:32Z

rebased around #846

crosbymichael · 2017-06-01T18:48:06Z

LGTM

mrunalp · 2017-06-01T18:49:49Z

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>

Mashimiao force-pushed the style-consistent-fix branch from a36389c to 265c768 Compare May 12, 2017 13:00

wking reviewed May 12, 2017

View reviewed changes

Mashimiao force-pushed the style-consistent-fix branch from 265c768 to f035b58 Compare May 13, 2017 01:05

wking reviewed May 13, 2017

View reviewed changes

Mashimiao force-pushed the style-consistent-fix branch from f035b58 to 7858d03 Compare May 13, 2017 10:31

TomSweeneyRedHat reviewed May 15, 2017

View reviewed changes

wking mentioned this pull request May 15, 2017

config-linux.md: add cgroupsPath entry and some modifications #823

Merged

Mashimiao force-pushed the style-consistent-fix branch 4 times, most recently from 204bb77 to 9bc1ef8 Compare May 23, 2017 07:51

consistency and style fix

b92cf90

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 June 1, 2017 16:31

mrunalp merged commit be3960a into opencontainers:master Jun 1, 2017

vbatts mentioned this pull request Jul 5, 2017

bump to v1.0.0-rc6 #882

Merged

wking mentioned this pull request Aug 31, 2017

config: Collapse extensibility to a single MUST #916

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

consistency and style fix #811

consistency and style fix #811

Mashimiao commented May 12, 2017

wking May 12, 2017

wking May 12, 2017

wking May 12, 2017

wking May 12, 2017

wking May 12, 2017

Mashimiao commented May 13, 2017

wking May 13, 2017

Mashimiao May 13, 2017

TomSweeneyRedHat May 15, 2017

wking May 15, 2017 •

edited

Loading

TomSweeneyRedHat May 15, 2017

Mashimiao commented May 23, 2017

Mashimiao commented Jun 1, 2017

crosbymichael commented Jun 1, 2017 •

edited by caniszczyk

Loading

mrunalp commented Jun 1, 2017 •

edited by caniszczyk

Loading

consistency and style fix #811

consistency and style fix #811

Conversation

Mashimiao commented May 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mashimiao commented May 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking May 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mashimiao commented May 23, 2017

Mashimiao commented Jun 1, 2017

crosbymichael commented Jun 1, 2017 • edited by caniszczyk Loading

mrunalp commented Jun 1, 2017 • edited by caniszczyk Loading

wking May 15, 2017 •

edited

Loading

crosbymichael commented Jun 1, 2017 •

edited by caniszczyk

Loading

mrunalp commented Jun 1, 2017 •

edited by caniszczyk

Loading