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

Change to plural forms for help output of docker service update #28147

Merged
merged 1 commit into from
Nov 8, 2016

Conversation

yongtang
Copy link
Member

@yongtang yongtang commented Nov 8, 2016

This fix is based on the comment in
#27567 (comment)

Basically, in the help output of docker service update, the --xxx-add flags typically have plural forms while --xxx-rm flags have singular forms.

This fix updates the help output for consistency.

This fix also updates the related docs in service_update.md.

The help output in service_update.md has been quite out-of-sync with the actual output, so this fix replaces the output with the most up-to-date output.

This fix is related to #27567.

cc @aaronlehmann

Signed-off-by: Yong Tang yong.tang.github@outlook.com

@AkihiroSuda
Copy link
Member

cc @mstanleyjones

@thaJeztah
Copy link
Member

Oh! Hm, I actually think the singular form is clearer, because each --something-add or --something-rm removes a single option, not multiple (but you can provide it multiple times)

This fix is based on the comment in
moby#27567 (comment)

Basically, in the help output of `docker service update`, the `--xxx-add`
flags typically have plural forms while `--xxx-rm` flags have singular
forms.

This fix updates the help output for consistency.

This fix also updates the related docs in `service_update.md`.
The help output in `service_update.md` has been quite out-of-sync
with the actual output so this fix replaces the output with the
most up-to-date output.

This fix is related to moby#27567.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the 27567-service-update-docs-plural-form branch from febfb7a to 1b42d9d Compare November 8, 2016 13:54
@yongtang
Copy link
Member Author

yongtang commented Nov 8, 2016

@thaJeztah Changed to singular form with consistency 😅

--restart-max-attempts uint64-ptr Maximum number of restarts before giving up (default none)
--restart-window duration-ptr Window used to evaluate the restart policy (default none)
--rollback Rollback to previous specification
--stop-grace-period duration-ptr Time to wait before force killing a container (default none)
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but we should change the output to be a bit more useful; duration-ptr is the internal implementation, and not very informative for the end-user.

Copy link
Member Author

Choose a reason for hiding this comment

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

The duration-ptr and uint64-ptr are defined in the Type() of the DurationOpt and Uint64Opt. I think -ptr could be removed as the purpose is to have nil as the default value.

Copy link
Member Author

Choose a reason for hiding this comment

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

@thaJeztah I added a PR #28163 for -ptr. Please take a look.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @yongtang!

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@mdlinville mdlinville left a comment

Choose a reason for hiding this comment

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

LGTM

@mdlinville
Copy link
Contributor

@thaJeztah is the janky failure spurious?

@thaJeztah
Copy link
Member

Let me restart janky

@vieux vieux merged commit fb9aca3 into moby:master Nov 8, 2016
@yongtang yongtang deleted the 27567-service-update-docs-plural-form branch November 8, 2016 21:26
yongtang added a commit to yongtang/docker that referenced this pull request Nov 9, 2016
This fix is based on the comment:
moby#28147 (comment)

Previously the output string of the `DurationOpt` is `duration-ptr`
and `Uint64Opt` is `uint64-ptr`. While it is clear to developers,
for a normal user `-ptr` might not be very informative.

On the other hand, the default value of `DurationOpt` and `Uint64Opt`
has already been quite informative: `none`. That means if no flag
provided, the value will be treated as none.
(like a ptr with nil as the default)

For that reason this fix removes the `-ptr`.

Also, the output in the docs of `service create` has been quite
out-of-sync with the true output. So this fix updates the docs
to have the most up-to-date help output of `service create --help`.

This fix is related to moby#28147.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
…-plural-form

Change to plural forms for help output of `docker service update`
vdemeester pushed a commit to vdemeester/docker-cli that referenced this pull request May 15, 2017
This fix is based on the comment:
moby/moby#28147 (comment)

Previously the output string of the `DurationOpt` is `duration-ptr`
and `Uint64Opt` is `uint64-ptr`. While it is clear to developers,
for a normal user `-ptr` might not be very informative.

On the other hand, the default value of `DurationOpt` and `Uint64Opt`
has already been quite informative: `none`. That means if no flag
provided, the value will be treated as none.
(like a ptr with nil as the default)

For that reason this fix removes the `-ptr`.

Also, the output in the docs of `service create` has been quite
out-of-sync with the true output. So this fix updates the docs
to have the most up-to-date help output of `service create --help`.

This fix is related to #28147.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
andrewhsu pushed a commit to docker-archive/docker-ce that referenced this pull request May 19, 2017
This fix is based on the comment:
moby/moby#28147 (comment)

Previously the output string of the `DurationOpt` is `duration-ptr`
and `Uint64Opt` is `uint64-ptr`. While it is clear to developers,
for a normal user `-ptr` might not be very informative.

On the other hand, the default value of `DurationOpt` and `Uint64Opt`
has already been quite informative: `none`. That means if no flag
provided, the value will be treated as none.
(like a ptr with nil as the default)

For that reason this fix removes the `-ptr`.

Also, the output in the docs of `service create` has been quite
out-of-sync with the true output. So this fix updates the docs
to have the most up-to-date help output of `service create --help`.

This fix is related to #28147.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Upstream-commit: 071c746e5ef5e407d6769a0fd436e8cf50b371a9
Component: cli
andrewhsu pushed a commit to docker-archive/docker-ce that referenced this pull request May 19, 2017
This fix is based on the comment:
moby/moby#28147 (comment)

Previously the output string of the `DurationOpt` is `duration-ptr`
and `Uint64Opt` is `uint64-ptr`. While it is clear to developers,
for a normal user `-ptr` might not be very informative.

On the other hand, the default value of `DurationOpt` and `Uint64Opt`
has already been quite informative: `none`. That means if no flag
provided, the value will be treated as none.
(like a ptr with nil as the default)

For that reason this fix removes the `-ptr`.

Also, the output in the docs of `service create` has been quite
out-of-sync with the true output. So this fix updates the docs
to have the most up-to-date help output of `service create --help`.

This fix is related to #28147.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Upstream-commit: 583dd837278ab8999a45ba9be109f706d57485ae
Component: cli
andrewhsu pushed a commit to docker-archive/docker-ce that referenced this pull request Jun 5, 2017
This fix is based on the comment:
moby/moby#28147 (comment)

Previously the output string of the `DurationOpt` is `duration-ptr`
and `Uint64Opt` is `uint64-ptr`. While it is clear to developers,
for a normal user `-ptr` might not be very informative.

On the other hand, the default value of `DurationOpt` and `Uint64Opt`
has already been quite informative: `none`. That means if no flag
provided, the value will be treated as none.
(like a ptr with nil as the default)

For that reason this fix removes the `-ptr`.

Also, the output in the docs of `service create` has been quite
out-of-sync with the true output. So this fix updates the docs
to have the most up-to-date help output of `service create --help`.

This fix is related to #28147.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Upstream-commit: ac4b6e35056b8a99993919411421e5a5fb2f30e9
Component: cli
dnephin pushed a commit to dnephin/cli that referenced this pull request Jun 14, 2017
This fix is based on the comment:
moby/moby#28147 (comment)

Previously the output string of the `DurationOpt` is `duration-ptr`
and `Uint64Opt` is `uint64-ptr`. While it is clear to developers,
for a normal user `-ptr` might not be very informative.

On the other hand, the default value of `DurationOpt` and `Uint64Opt`
has already been quite informative: `none`. That means if no flag
provided, the value will be treated as none.
(like a ptr with nil as the default)

For that reason this fix removes the `-ptr`.

Also, the output in the docs of `service create` has been quite
out-of-sync with the true output. So this fix updates the docs
to have the most up-to-date help output of `service create --help`.

This fix is related to #28147.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants