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

Remove range limit #783

Merged
merged 1 commit into from
May 9, 2017
Merged

Conversation

zhouhao3
Copy link

Fixes #780
Signed-off-by: zhouhao zhouhao@cn.fujitsu.com

Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
@@ -458,7 +458,7 @@ type WindowsMemoryResources struct {
type WindowsCPUResources struct {
// Number of CPUs available to the container.
Count *uint64 `json:"count,omitempty"`
// CPU shares (relative weight to other containers with cpu shares). Range is from 1 to 10000.
// CPU shares (relative weight to other containers with cpu shares).
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to wait on this until #777 lands, because the Windows folks are sounding like they'd like to return the explicit ranges to their settings (here and here). On the other hand, there's no need to list the range in the Go comments even if the Markdown spec gives a valid range. If we want to remove range comments from the Go types regardless of whether the spec gives a valid range, we can go ahead with this removal without waiting on #777.

@hqhq
Copy link
Contributor

hqhq commented May 9, 2017

LGTM

Approved with PullApprove

1 similar comment
@crosbymichael
Copy link
Member

crosbymichael commented May 9, 2017

LGTM

Approved with PullApprove

@crosbymichael crosbymichael merged commit 6a17a38 into opencontainers:master May 9, 2017
@zhouhao3 zhouhao3 deleted the range-limt branch May 10, 2017 01:16
@vbatts vbatts mentioned this pull request Jul 5, 2017
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