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

Reverse comments in cli_args to fix rm param. #48

Merged
merged 1 commit into from
Jan 26, 2014
Merged

Reverse comments in cli_args to fix rm param. #48

merged 1 commit into from
Jan 26, 2014

Conversation

realloc
Copy link
Contributor

@realloc realloc commented Jan 22, 2014

Without this patch rm attribute doesn't work. Docker (0.7.6) does not accept just '-rm'.
I'm not sure if this breaks anything else.

@bflad
Copy link
Contributor

bflad commented Jan 26, 2014

For which command/action: build or run?

This may be a good idea to be explicit with the =true/=false though. I'm open to accept this. We may also want to consider getting away from using shell CLI and opt for a real library/direct interface to avoid these sorts of issues.

I'm curious if others can weigh in on this issue or try suggested patch. Lacking a full regression suite for this cookbook (mea culpa), its hard to know the impact of changing this logic.

@bflad
Copy link
Contributor

bflad commented Jan 26, 2014

I read into the docker code more and found their CLI flag handling. Specifically I see: https://github.com/dotcloud/docker/blob/master/pkg/mflag/flag.go#L47-L48

I'll accept this, fix the Rubocop warning, and release in 0.27.0.

bflad added a commit that referenced this pull request Jan 26, 2014
Reverse comments in cli_args to fix rm param.
@bflad bflad merged commit f0ad76a into sous-chefs:master Jan 26, 2014
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.

2 participants