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 pointers for slices preferring omitempty tag instead #316

Merged
merged 1 commit into from
Jan 26, 2016

Conversation

mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Jan 26, 2016

Converting to pointers to be consistent with other cgroups settings.

Signed-off-by: Mrunal Patel mrunalp@gmail.com

@mrunalp mrunalp changed the title Add pointers to couple of cgroups settings Convert couple of cgroups settings to pointers Jan 26, 2016
@mrunalp
Copy link
Contributor Author

mrunalp commented Jan 26, 2016

@crosbymichael @vishh PTAL

@mrunalp
Copy link
Contributor Author

mrunalp commented Jan 26, 2016

We can wait for #284 to merge before this.

@@ -195,7 +195,7 @@ type Network struct {
// Set class identifier for container's network packets
ClassID *uint32 `json:"classID"`
// Set priority of network traffic for container
Priorities []InterfacePriority `json:"priorities"`
Priorities []*InterfacePriority `json:"priorities"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? If we add an omitempty tag, even an empty list signifies non-existence of this field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vishh I am fine one way or the other. But we have them as pointers in more places compared to where we don't.

Copy link
Contributor

Choose a reason for hiding this comment

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

In general, we don't need pointers for slices and maps. I can go cleanup
the Spec again once I get some spare cycles.

On Mon, Jan 25, 2016 at 4:46 PM, Mrunal Patel notifications@github.com
wrote:

In runtime_config_linux.go
#316 (comment):

@@ -195,7 +195,7 @@ type Network struct {
// Set class identifier for container's network packets
ClassID *uint32 json:"classID"
// Set priority of network traffic for container

  • Priorities []InterfacePriority json:"priorities"
  • Priorities []*InterfacePriority json:"priorities"

@vishh https://github.com/vishh I am fine one way or the other. But we
have them as pointers in more places compared to where we don't.


Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/316/files#r50781540.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vishh No issues going one way or the other. I can change the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks :)

On Mon, Jan 25, 2016 at 5:03 PM, Mrunal Patel notifications@github.com
wrote:

In runtime_config_linux.go
#316 (comment):

@@ -195,7 +195,7 @@ type Network struct {
// Set class identifier for container's network packets
ClassID *uint32 json:"classID"
// Set priority of network traffic for container

  • Priorities []InterfacePriority json:"priorities"
  • Priorities []*InterfacePriority json:"priorities"

@vishh https://github.com/vishh No issues going one way or the other. I
can change the PR.


Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/316/files#r50783105.

@@ -195,7 +195,7 @@ type Network struct {
// Set class identifier for container's network packets
ClassID *uint32 `json:"classID"`
// Set priority of network traffic for container
Priorities []InterfacePriority `json:"priorities"`
Priorities []*InterfacePriority `json:"priorities,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

For this PR just adding omitempty here should do. Kindly revert the pointer changes.

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp mrunalp changed the title Convert couple of cgroups settings to pointers Remove pointers for slices preferring omitempty tag instead Jan 26, 2016
@mrunalp
Copy link
Contributor Author

mrunalp commented Jan 26, 2016

@vishh Updated to remove pointers from slices.

@vishh
Copy link
Contributor

vishh commented Jan 26, 2016

LGTM. Thanks for the cleanup @mrunalp 👍

@hqhq
Copy link
Contributor

hqhq commented Jan 26, 2016

LGTM

vishh added a commit that referenced this pull request Jan 26, 2016
Remove pointers for slices preferring omitempty tag instead
@vishh vishh merged commit 07bce39 into opencontainers:master Jan 26, 2016
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.

3 participants