config: Synchronize comments between Markdown and Go #525
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Catch up with the spec title from faad7e0 (README: title rename, 2016-04-04, opencontainers#365). Also make the Go comment consistent with the Markdown spec (no need to maintain two phrasings for the same idea). The only difference between the phrasings is now some shuffling at the beginning to start off with the property name (to keep godoc happy). The JSON Schema entry (in defs.json) is different too, because it has to apply to both the configuration and state JSON, so mentioning "bundle" makes less sense than mentioning "document". Signed-off-by: W. Trevor King <wking@tremily.us>
Don't mention OS and Arch, since they're covered by the list (in Markdown) and Platform struct (in Go). This gives us one less place to update if we ever change the schema for the platform object. Signed-off-by: W. Trevor King <wking@tremily.us>
I've dropped "main process" because "container process" is currently a much more popular way of identifying that process in this specification. Before this commit: $ git grep -i 'main process' | wc -l 4 $ git grep -i 'container process' | wc -l 13 I've also added our usual: (<type>, <required|optional>) to the Markdown so folks can see that this is a required object. Signed-off-by: W. Trevor King <wking@tremily.us>
We use both forms, but the latter was more popular. Before this branch: $ git grep -i 'container process' origin/master | wc -l 13 $ git grep -i 'main process' origin/master | wc -l 4 Also fix "process id" -> "process ID" in one of the lines I touched, to match fork(2) [1]. [1]: http://man7.org/linux/man-pages/man2/fork.2.html Signed-off-by: W. Trevor King <wking@tremily.us>
I've also added our usual: (<type>, <required|optional>) to the Markdown so folks can see that this is a required object. Signed-off-by: W. Trevor King <wking@tremily.us>
I've changed the old "as it is accessible to ..." to the more compact "as seen by ..." language from the old Markdown version, although I don't think it's strictly necessary. The original "accessbile to" language is from 77d44b1 (Update runtime.md, 2015-06-16), which actually looked fairly similar to the language I'm using here. That commit's "hostname for the container" lanuage went away in 7ac41c6 (config.md: reformat into a standard style, 2015-06-30), although that commit made too many changes to motivate them all at that level. I've left that language out of the Go comment, because truncating for compact Go comments is fine (the Markdown entry is canonical, and the Go comment is just to provide some minimal context). Signed-off-by: W. Trevor King <wking@tremily.us>
I've replaced the old MAY with our usual (<type>, <optional|required>) to get the property name first, since that translates more directly into a Go comment that godoc will like. Signed-off-by: W. Trevor King <wking@tremily.us>
The new wording isn't particularly close to either of the old wordings, but it reads more clearly to me. I've also added our usual: (<type>, <required|optional>) to the Markdown so folks can see that this is an optional object (although see [1] for a more complete version). [1]: opencontainers#427 Subject: config: Explicitly list 'hooks' as optional Signed-off-by: W. Trevor King <wking@tremily.us>
I've replaced the old OPTIONAL with our usual: (<type>, <optional|required>) to get the property name first, since that translates more directly into a Go comment that godoc will like. The new Go comment is much shorter, dropping "unstructured" (because the Markdown says "structured or unstructured") and "set by external tools..." (because *everything* in the configuration JSON is set by external-to-the-runtime tools). Signed-off-by: W. Trevor King <wking@tremily.us>
|
Additional details for any specific wording change are in the commit messages themselves. |
|
LGTM |
1 similar comment
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Spun off from here, this unifies the Markdown and Go comments for all of
Specexcept theLinuxandSolarisentries, because it's better to not have to maintain different descriptions for the same idea. The series also makes a few knock-on changes for consistency.There seemed like enough was going on after making
Specconsistent, so I'm postponing consistency for other Go types until after this lands.This PR will conflict with a number of long-open PRs that have more complete fixes for some of these issues (e.g. #427), but those commits have mostly sat unreviewed for months, so I'm hoping the more restricted pivots here have a better chance of landing. I can rebase any of the conflicted PRs (that I've submitted anyway) around this if/when it lands.