Clarity for commands vs global options

[mikebrow]

While reviewing the global options section in runtime.md, it seemed additional clarity was needed for the command and global options requirements. Discussed and worked on wording with @wking via private IRC. Also adds uppercase RFC 2119 words for this section.

With regard to the statement Command names MUST not start with hyphens, the rationale behind this decision is to "distinguish unrecognized commands from unrecognized options, and because we are "requiring runtimes to fail-fast for unrecognized commands" [1,2].

[1] https://github.com/wking/oci-command-line-api/pull/8/files#r46898167
[2] 527f3c6#commitcomment-14835617

Signed-off-by: Mike Brown [email protected]

[wking]

This looks like a good step forward :). I left a few comments inline, but feel free to push back if the suggestions don't seem sound.

[mikebrow]

Good comments. I've addressed them. I've also added use of upper case MAY where you also used upper case on your wip branch.

Cheers!

[wking]

I'm not sure about this MUST. It's a requirement we're imposing on this specification so runtime authors know what to expect, and not a requirement that we're imposing on the runtime. On the other hand, I think folks will generally get the idea even if we don't get this quite right ;). Do you have a feeling for which way would be right?

[mikebrow]

I think this works. You're not saying someone can't do something like a command with a hyphenated global option. You're say, instead, that the spec has certain commands that are not hyphenated and they follow the global options.

[wking]

On Mon, Dec 07, 2015 at 03:34:01PM -0800, Mike Brown wrote:

+Command names MUST not start with hyphens.

… You're say, instead, that the spec has certain commands that are
not hyphenated and they follow the global options.

Right, so what's MUST about it? There's no way runtime implementors
could get that wrong, since it's a requirement we're making of the
spec itself. See also my suggested test-case criterion 1.

[mikebrow]

You're saying '-START' is not the same as 'START' wrt compliance. Rather -START path in front of the command START would be a global option. You're also limiting the spec writers from muddying commands and global options. You're forcing them to have a command on every invoke.

[wking]

On Mon, Dec 07, 2015 at 03:54:55PM -0800, Mike Brown wrote:

+Command names MUST not start with hyphens.

You're saying -START is not the same as START wrt compliance.

No, I'm saying:

The only commands we define now are ‘version’, ‘start’, ‘exec’,
‘pause’, ‘resume’, and ‘signal’. We may release a future version of
this spec with a command like ‘checkpoint’, but will not release any
future versions of this spec with a command like ‘-checkpoint’.

Without a pattern like “commands will not start with a hyphen” it
would be harder to distinguish unrecognized commands from unrecognized
options, and we're requiring runtimes to fail-fast for unrecognized
commands.

I agree that ‘-start’ and ‘start’ are not the same, but I think that's
clear without this sentence.

[wking]

On Mon, Dec 07, 2015 at 04:52:47PM -0800, Mike Brown wrote:

Your statements clarified for me how you read them, specifically
with the future guarantees and the purpose of making easier to
parse. I don't think we need to add any statements about future
revisions or the rationale for the requirement. It's the pattern
selected .. so it is what it is.

Works for me. Even if we don't change the spec text, I think it's
worth being very explicit about the rationale and our intentions in
the commit message. That way we don't have to rehash the reasoning if
we do decide that the spec needs clarification later. Can you update
the commit message with something to cover the points made in [1,2]?

[mikebrow]

Good point done :-)

[wking]

On Mon, Dec 07, 2015 at 09:30:38PM -0800, Mike Brown wrote:

+Command names MUST not start with hyphens.

Good point done :-)

But not force-pushed? I'm still seeing d67df10 anyway.

[mikebrow]

Yeah I just put it in the PR comment. I refreshed the commit. Not used to putting long winded commit messages in. Cheers.

[wking]

On Mon, Dec 07, 2015 at 10:13:31PM -0800, Mike Brown wrote:

Yeah I just put it in the PR comment. I refreshed the commit. Not
used to putting long winded commit messages in.

2b084a3 LGTM. I'm fine with it landing as is, but here are a few
notes on the commit message:

Discussed and worked on wording with @wking via private IRC.

This^ isn't important enough to include in the commit message. Folks
reading this will care about what changed, what our motivation was,
and why we implemented it the way we did. They don't care who we were
(except for copyright ownership, and Git's Author: covers that) or
what channel we used for the discussion. I'll my Reviewed-by when
this lands, so folks will know that I signed-off on it (and we'll add
Reviewed-by tags for anyone else that LGTMs it too).

"distinguish unrecognized commands…

This^ is missing a closing quote.

[1] https…

No need to escape these^ references (e.g. see 3606bcf). Git's command
line tools don't markup the message, and GitHub links URLs but doesn't
render Markdown in the messages.

For motivating long-winded commit messages, see [1,2]. I like them
because you can go straight from ‘git blame’ to notes motivating that
line, instead of going blame → commit hash → GitHub PR → some comment
in that discussion. And I have a local mirror of the commits (thanks
Git :), but I don't keep a local mirror of the GitHub PR discussion.

[wking]

Just checking up on outstanding PRs, and thought I'd file a note here
saying that we're a day out since 2b084a3 was pushed 1 and I've
LGTMed it 2. I'll land it after the two-worday window wraps up
tomorrow unless anyone else raises issues with it.