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

Un-deprecated command line short variant options of -c #22621

Conversation

yongtang
Copy link
Member

@yongtang yongtang commented May 10, 2016

Since 1.9, the following short variant options have been deprecated in favor of their long variants:
docker run -c (--cpu-shares)
docker build -c (--cpu-shares)
docker create -c (--cpu-shares)
docker update -c (--cpu-shares)

However, -c is still widely used and is considered as a convenient option for swarm (see #16271).

This fix undeprecated the command line short variant options of -c and updated the deprecated.md.

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

@@ -661,7 +661,7 @@ container:
| `--memory-swap=""` | Total memory limit (memory + swap, format: `<number>[<unit>]`). Number is a positive integer. Unit can be one of `b`, `k`, `m`, or `g`. |
| `--memory-reservation=""` | Memory soft limit (format: `<number>[<unit>]`). Number is a positive integer. Unit can be one of `b`, `k`, `m`, or `g`. |
| `--kernel-memory=""` | Kernel memory limit (format: `<number>[<unit>]`). Number is a positive integer. Unit can be one of `b`, `k`, `m`, or `g`. Minimum is 4M. |
| `-c`, `--cpu-shares=0` | CPU shares (relative weight) |
| `--cpu-shares=0` | CPU shares (relative weight) |
Copy link
Member

Choose a reason for hiding this comment

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

nit: can you re-align the | here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @thaJeztah I just updated the PR.

@thaJeztah
Copy link
Member

LGTM, thanks!

@runcom
Copy link
Member

runcom commented May 10, 2016

LGTM (besides @thaJeztah's nit)

@cpuguy83
Copy link
Member

:( I don't think this is a good idea.

@thaJeztah
Copy link
Member

There was some discussion about this on the original PR #16271, so ping @vieux @abronan are there still objections against this?

If we decide to not remove these, we should un-deprecate them at least

@yongtang yongtang force-pushed the 05092016-remove-deprecated-command-line-short-variant-options branch from bd12bbd to 8954b72 Compare May 10, 2016 14:13
@yongtang
Copy link
Member Author

@runcom @cpuguy83 @thaJeztah Thanks for the review. Let me know if we want to un-deprecate this option so that I could update the PR.

@LK4D4
Copy link
Contributor

LK4D4 commented May 13, 2016

If @cpuguy83 doesn't like it, then probably we shouldn't do it :/

@thaJeztah
Copy link
Member

Had a quick chat with @cpuguy83 and he mentioned seeing a lot of people still using the shorthand -c option (despite the deprecation warning 😞). I guess we should take the sensible approach then, and un-deprecate the shorthand -c. It's only a minor cleanup, so

@yongtang can you update the PR to un-deprecate?

Since 1.9, the following short variant options have been
deprecated in favor of their long variants:
`docker run -c (--cpu-shares)`
`docker build -c (--cpu-shares)`
`docker create -c (--cpu-shares)`
`docker update -c (--cpu-shares)`

However, `-c` is still widely used and is considered as
a convenient option for swarm (see moby#16271).

This fix undeprecated the command line short
variant options of `-c` and updated the deprecated.md.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the 05092016-remove-deprecated-command-line-short-variant-options branch from 8954b72 to fea7acf Compare May 26, 2016 15:23
@yongtang yongtang changed the title Remove deprecated command line short variant options Un-deprecated command line short variant options of -c May 26, 2016
@yongtang
Copy link
Member Author

yongtang commented May 26, 2016

Thanks @thaJeztah. The pull request has been updated. Let me know if there are any issues.

Note: I also updated the comment and title section of this pull request.

@thaJeztah
Copy link
Member

LGTM, thanks @yongtang

@icecrime
Copy link
Contributor

LGTM

@thaJeztah
Copy link
Member

docs LGTM as well. Merging

@thaJeztah thaJeztah merged commit 4a031f1 into moby:master May 27, 2016
@yongtang yongtang deleted the 05092016-remove-deprecated-command-line-short-variant-options branch May 28, 2016 14:55
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.

7 participants