8000 Specifying -a and -d to docker run should throw an error by aanm · Pull Request #10421 · moby/moby · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Specifying -a and -d to docker run should throw an error #10421

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 6, 2015
Merged

Specifying -a and -d to docker run should throw an error #10421

merged 1 commit into from
Feb 6, 2015

Conversation

aanm
Copy link
Contributor
@aanm aanm commented Jan 28, 2015

The cmd.Lookup should be "-attach" and not "attach", as seen in
docker/docker/runconfig/parse.go

Fixes #10388

Signed-off-by: André Martins martins@noironetworks.com

@duglin
Copy link
Contributor
duglin commented Jan 29, 2015

@aanm fix looks like the right one. Could you add a testcase for this? I see several tests in integration-cli for "-a" but none for "--attach" - but it should be easy enough to just dup one.

@duglin
Copy link
Contributor
duglin commented Jan 30, 2015

Can you add
Fxed #10388
or
Closes #10388
to your original comment so github will close the issue automatically?

@LK4D4
Copy link
Contributor
LK4D4 commented Jan 30, 2015

Also commits look weird. Could you squash-rebase pls?

@aanm aanm force-pushed the 10388-Specifying_confliting_options branch from 3bc771a to 5ee3253 Compare January 31, 2015 02:00
@aanm aanm changed the title Fixes #10388 Specifying -a and -d to docker run should throw an error Jan 31, 2015
@aanm
Copy link
Contributor Author
aanm commented Jan 31, 2015

@duglin Done
@LK4D4 Sorry about that, let me know if everything is alright now

duglin
duglin reviewed Jan 31, 2015
View reviewed changes
integration-cli/docker_cli_run_test.go
t.Fatalf("Should have been returned an error with conflicting options -a and -d")
}

deleteAllContainers()
Copy link
Contributor

Choose a reason for hiding this comment

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

can you move this to the top and make it a defer

@aanm
Fixes moby#10388
af90fe6 
The cmd.Lookup should be "-attach" and not "attach", as seen in
docker/docker/runconfig/parse.go

Signed-off-by: André Martins <martins@noironetworks.com>
@aanm aanm force-pushed the 10388-Specifying_confliting_options branch from 5ee3253 to af90fe6 Compare February 2, 2015 12:08
@LK4D4
Copy link
C 8000 ontributor
LK4D4 commented Feb 6, 2015

@aanm Could you give next time more descriptive commit message title?
LGTM

@crosbymichael
Copy link
Contributor

LGTM

crosbymichael added a commit that referenced this pull request Feb 6, 2015
@crosbymichael
Merge pull request #10421 from noironetworks/10388-Specifying_conflit…
eb79acd 
…ing_options

Specifying -a and -d to docker run should throw an error
@crosbymichael crosbymichael merged commit eb79acd into moby:master Feb 6, 2015
@SvenDowideit SvenDowideit mentioned this pull request Feb 9, 2015
Closed
@aanm aanm deleted the 10388-Specifying_confliting_options branch February 19, 2015 15:34
@SvenDowideit SvenDowideit mentioned this pull request Feb 26, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Specifying -a and -d to docker run should throw an error
4 participants
@aanm @duglin @LK4D4 @crosbymichael
0