Skip to content
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

fix hooks man and help #191

Merged
merged 1 commit into from
Oct 17, 2016

Conversation

Mashimiao
Copy link

Signed-off-by: Ma Shimiao [email protected]

Path to command to run in poststart hooks. This command will be run before
the container process gets launched but after the container environment and
main process has been created.
**--poststart**=[CMD[:ARG]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the brackets around the whole thing? I'd expect:

--poststart=CMD[:ARG...]

or some such. Unless we update --poststart so --poststart= clears any existing post-start hooks.

On the other hand, maybe not worth sinking much time into this with opencontainers/runtime-spec#483 presumably being merged soon.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 08/17/2016 11:01 AM, W. Trevor King wrote:

Why the brackets around the whole thing? I'd expect:

--poststart=CMD[:ARG...]

brackets around the whole thing means it accepts more than one, it's a slice.
--poststart=CMD[:ARG...]
looks like only ARG accepted as slice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Tue, Aug 16, 2016 at 08:07:50PM -0700, Ma Shimiao wrote:

08/17/2016 11:01 AM, W. Trevor King:

Why the brackets around the whole thing? I'd expect:

--poststart=CMD[:ARG...]

brackets around the whole thing means it accepts more than one, it's
a slice.

--poststart=CMD[:ARG...]

looks like only ARG accepted as slice.

I'm used to brackets in man pages meaning “the enclosed bit is
optional” with “accepts more than one” being represented with an
ellipsis. For example:

$ man git-commit | head -n12 | tail -n6
git commit [-a | --interactive | --patch] [-s] [-v] [-u] [--amend]
[--dry-run] [(-c | -C | --fixup | --squash) ]
[-F | -m ] [--reset-author] [--allow-empty]
[--allow-empty-message] [--no-verify] [-e] [--author=]
[--date=] [--cleanup=] [--[no-]status]
[-i | -o] [-S[]] [--] [...]

Showing that is an optional argument for -S and can be
repeated.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--bind=[[HOST-DIR:CONTAINER-DIR][:OPTIONS]]
bind also used brackets around all. Based on your opinion, it's also need to fix?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Tue, Aug 16, 2016 at 08:41:17PM -0700, Ma Shimiao wrote:

--bind=[[HOST-DIR:CONTAINER-DIR][:OPTIONS]]
bind also used brackets around all. Based on your opinion, it's also need to fix?

Yeah, I'd rather see:

--bind=SOURCE:DESTINATION[:OPTIONS]

with text in the description saying something like:

--bind may be set multiple times to configure multiple bind mounts.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@Mashimiao Mashimiao force-pushed the man-and-help-fix-hooks branch from 3902084 to 97fc2fa Compare August 17, 2016 05:39
**--prestart**=CMD
Path to command to run in prestart hooks. This command will be run before
the container process gets launched but after the container environment.
**--poststart**=CMD[:ARGS..]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.....

@Mashimiao Mashimiao force-pushed the man-and-help-fix-hooks branch from 97fc2fa to 41fb3fc Compare August 18, 2016 01:03
@Mashimiao
Copy link
Author

ping @mrunalp @liangchenye

is stopped.

**--prestart**=CMD[:ARGS...]
Set commands to run in prestart hooks. Can be specified multiple times.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/commands/command

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrunalp fixed.

@Mashimiao Mashimiao force-pushed the man-and-help-fix-hooks branch from 41fb3fc to a66cd0d Compare September 7, 2016 01:17
@Mashimiao Mashimiao force-pushed the man-and-help-fix-hooks branch from a66cd0d to 237e4ae Compare September 22, 2016 02:13
@Mashimiao
Copy link
Author

PR rebased.
Ping @mrunalp @liangchenye

@liangchenye
Copy link
Member

LGTM , but still need rebase...

Signed-off-by: Ma Shimiao <[email protected]>
@Mashimiao Mashimiao force-pushed the man-and-help-fix-hooks branch from 237e4ae to 2f7d834 Compare October 17, 2016 08:52
@Mashimiao
Copy link
Author

@liangchenye rebased

@mrunalp
Copy link
Contributor

mrunalp commented Oct 17, 2016

LGTM

@mrunalp mrunalp merged commit ad632ee into opencontainers:master Oct 17, 2016
@Mashimiao Mashimiao deleted the man-and-help-fix-hooks branch November 14, 2016 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants