-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Support volumes propagation flags (Issue 554) #705
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
Conversation
| */ | ||
| public enum PropagationMode { | ||
| /** default */ | ||
| mount_default(""), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name as java convention and annotate with JsonProperty according to real field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem is that the values are default and private, which is not allowed. So how should they be called then?
Do you have an example for the JsonProperty stuff? I tried to make this analogue to SELContext.java.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed the values but could not figure out where the JsonProperty annotation should go here. My code should be in line with AccessMode and SELContext.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please let me know if anything is missing. If something about Json is required then please let me know what exactly, ideally with a pointer to other similar code. Would be great if a version with this change could be released soon.
|
LGTM but will back on weekend, sorry. |
|
#696 is the same options right? Could you review each other PR? |
|
I had based my change on the branch for the other fix. So my branch contains the the same commit. |
|
@Hendrik-H please cherry-pick again that initial commit from #696 as it contains an integration test which is missing in your case. @KostyaSha this PR would be more complete in terms of features. However, I believe you could have pointed out to both of us that we 're doing the same thing in parallel while you were reviewing both PRs at the same time |
|
Hi, I can only look into it on Monday. From what I had seen so far we added different things in the same area. Can't currently check the code. I thought one could first pull in the other change and then mine as I had based mine in the earlier commits. On September 30, 2016 7:36:23 AM GMT+02:00, Dimitris Mandalidis [email protected] wrote:
|
|
I rebased my change now on top of the latest version of https://github.com/dmandalidis/docker-java/tree/issue-688. I'm still a bit new to git, so hope to have done it right ... |
|
Seems ok, please fixup your latest commit with the one that introduced the mistake. |
|
sorry, no idea how I would do that. looks like I had the wrong version in 5abe479. so would somehow have to redo the whole thing. what's the problem of an additional commit ? |
|
5abe479 introduced the wrong version. PR would be more clear that way, but again it's just my opinion :) See manual of git-rebase (along with the interactive option) |
|
ok, I did a |
|
@KostyaSha anything missing to get this merged? |
* Support nocopy option in volume binds (docker-java#688) * support volumes propagation flags, fixes docker-java#554 * renamed values to match java convention
* Support nocopy option in volume binds (docker-java#688) * support volumes propagation flags, fixes docker-java#554 * renamed values to match java convention
The code change is based on the code for issue #688, which is about the same code area.
This change is