-
Notifications
You must be signed in to change notification settings - Fork 553
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
style: Collect established conventions in a discoverable location #287
Conversation
Do we want to put scoping decisions in here (e.g. network setup beyond namespace creation is out-of-scope for this spec)? Or are those going to be recorded somewhere else? |
Add the pull-request tag now that I've filed [1]. I'm not sure if the feature tag fits all that well, but we need it to show up in the current status-config.json filters. [1]: opencontainers/runtime-spec#287
While tagging open issues from the mailing list, I re-discovered my post asking about the “OCI Scope Table” referenced in the adopted charter. Until we get more clarity on what that table contained, landing any scoping language via this PR is probably premature. For now, I think we should focus on the style issues like those discussed in c41523b (this PR's current commit). |
|
||
## Traditionally hex settings should use JSON integers, not JSON strings | ||
|
||
The config JSON isn't enough of a UI to be worth jumping through string ↔ integer hoops to support an 0x… form ([source][integer-over-hex]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps this could be clearer as to what the expectation is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Tue, Jan 05, 2016 at 07:56:12AM -0800, Vincent Batts wrote:
+The config JSON isn't enough of a UI to be worth jumping through string ↔ integer hoops to support an 0x… form ([source][integer-over-hex]).
perhaps this could be clearer as to what the expectation is.
Sure. In c41523b → 408412b, I've called out network.classID as an
example (although it's currently using a string. If we accept the
integer style that seems popular in 1, I'll file a PR updating
classID).
So we have something to cite to avoid rehashing established decisions. Provide some motivation and links to the backing discussion so folks can re-open these if they have new information that wasn't covered in the original decision. Like the glossary (1873498, glossary: Provide a quick overview of important terms, 2015-08-11, opencontainers#107), I've used subsection titles for each entry to get link anchors. Signed-off-by: W. Trevor King <wking@tremily.us>
sure, lgtm |
LGTM |
style: Collect established conventions in a discoverable location
Thanks for this PR @wking ! |
The just-landed style conventions prefer integers to hex strings [1], and I said I'd post an update for this setting if/when those landed [2]. The kernel uses uint32s for this setting [3]. [1]: opencontainers#287 [2]: opencontainers#287 (comment) [3]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/net/cls_cgroup.h?id=refs/tags/v4.3#n24 Signed-off-by: W. Trevor King <wking@tremily.us>
My pull request landed on 2015-01-08 in 6aa53ed [1], although it missed the 0.2.0 tag [2]. Scoping is still waiting on feedback about "OCI Scope Table" referenced in the adopted charter [3]. [1]: opencontainers/runtime-spec#287 (comment) [2]: opencontainers/runtime-spec#294 (comment) [3]: Message-ID: <20151208201013.GF2767@odin.tremily.us> Subject: Re: OCI News (scoping) Date: Tue, 8 Dec 2015 12:10:13 -0800
The just-landed style conventions prefer integers to hex strings [1], and I said I'd post an update for this setting if/when those landed [2]. The kernel uses uint32s for this setting [3]. [1]: opencontainers#287 [2]: opencontainers#287 (comment) [3]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/net/cls_cgroup.h?id=refs/tags/v4.3#n24 Signed-off-by: W. Trevor King <wking@tremily.us>
So we have something to cite to avoid rehashing established decisions.
Provide some motivation and links to the backing discussion so folks
can re-open these if they have new information that wasn't covered in
the original decision.
Like the glossary (1873498, glossary: Provide a quick overview of
important terms, 2015-08-11, #107), I've used subsection titles for
each entry to get link anchors.
I'm fine dropping any of the decisions from this PR if they turn out
to be contentious, and will happily add any that I missed if folks
bring them to my attention here.
Mailing-list suggestion here, and I'm interpreting the absence of
feedback there, plus a maintainer proposing the same thing in #273 as
enough of a consensus to transition this idea into a PR. If there is
pushback on the idea of listing policy/style decisions at all, we
should probably close this PR and post the pushback reason in the
mailing-list thread for further discussion.
Fixes #273.