Skip to content

Conversation

@hqhq
Copy link
Contributor

@hqhq hqhq commented Jan 15, 2016

Fixes: moby/moby#19329

Signed-off-by: Qiang Huang [email protected]

@cyphar
Copy link
Member

cyphar commented Jan 15, 2016

Could this cause issues with Resouces no longer being a pointer? Or is the cgroup struct always a pointer anyway?

Copy link
Contributor

Choose a reason for hiding this comment

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

I sort of don't understand why we need to embed it? Why just don't use value instead of pointer?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, sorry, it's for backward compatibility.

@mrunalp
Copy link
Contributor

mrunalp commented Jan 15, 2016

Hmm, the idea behind making Resources a pointer was that in the future we can clean up the API so if Resources are nil, we can just join instead of create.

@LK4D4
Copy link
Contributor

LK4D4 commented Jan 15, 2016

@mrunalp I think we can embed pointer.

@mrunalp
Copy link
Contributor

mrunalp commented Jan 15, 2016

Cool. I would prefer that.

Sent from my iPhone

On Jan 15, 2016, at 12:53 PM, Alexander Morozov [email protected] wrote:

@mrunalp I think we can embed pointer.


Reply to this email directly or view it on GitHub.

@hqhq hqhq force-pushed the hq_embed_resource branch from 2da0993 to f048eaf Compare January 19, 2016 12:07
@hqhq
Copy link
Contributor Author

hqhq commented Jan 19, 2016

@mrunalp @LK4D4 Updated.

@crosbymichael
Copy link
Member

LGTM

mrunalp pushed a commit that referenced this pull request Jan 19, 2016
Embed Resources for backward compatibility
@mrunalp mrunalp merged commit e91b055 into opencontainers:master Jan 19, 2016
@mrunalp
Copy link
Contributor

mrunalp commented Jan 19, 2016

LGTM

@hqhq hqhq deleted the hq_embed_resource branch January 20, 2016 01:15
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.

6 participants