Skip to content

Use a separate "execute" for running bash commands in containers #100

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

Merged
merged 19 commits into from
Aug 15, 2017

Conversation

apurvis
Copy link
Contributor

@apurvis apurvis commented Aug 15, 2017

I decided that having the --command flag on the bash command made the situation more complicated than it should be, especially when introducing the --all switch, so i separated execute to its own command namespace. I think it's better this way.

Also addresses a bug where Open3 would block an interactive session (bug is on master but was not released)


execute.desc 'bash command to run (wrap argument in quotes)'
execute.arg_name 'BASH_COMMAND'
execute.flag [:c, :command], type: String, required: true

Choose a reason for hiding this comment

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

Use %i or %I for an array of symbols.

@apurvis
Copy link
Contributor Author

apurvis commented Aug 15, 2017

(we already had run, but that spins up a new container just to run one command... anyone got any ideas better than execute to differentiate the two?)

Copy link

@dtboctor dtboctor left a comment

Choose a reason for hiding this comment

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

Whatever is fine, it was fine before in my opinion but whatever you think is clear.

@apurvis apurvis merged commit a2a87ee into master Aug 15, 2017
@apurvis apurvis deleted the always_tty branch August 15, 2017 18:21
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