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

`hooks[].command` should be equivalent to `commands[].command` #730

Open
LukasGentele opened this issue Oct 5, 2019 · 4 comments
Open

`hooks[].command` should be equivalent to `commands[].command` #730

LukasGentele opened this issue Oct 5, 2019 · 4 comments

Comments

@LukasGentele
Copy link
Member

@LukasGentele LukasGentele commented Oct 5, 2019

Is your feature request related to a problem?
Currently, commands[].command allows a string with bash-like syntax that is interpreted like a shell script. hooks[].command, however, expects a single binary and allows to provide args via hooks[].args. This is not intuitive to understand.

Which solution do you suggest?
hooks[].command should be equivalent to commands[].command and expect a command and hooks[].args should be deprecated. If hooks[].args is specified, there could be a fallback to the current behavior which prevents a breaking change but allows hooks[].command to adapt the new behavior.

Which alternative solutions exist?

Additional context

/kind feature

@mgruener
Copy link
Contributor

@mgruener mgruener commented Nov 7, 2019

Why not allow for both variants? Add commands[].args. If it is present, treat commands[].command as single binary and call it with the args in commands[].args and if commands[].args is absent, interpret commands[].command as shell script. Same for hooks[].

If that gets too complex, I would prefer keeping the .args variant and adding it to commands[], as this is more readable for commands / hooks with more than a few parameters.

@LukasGentele
Copy link
Member Author

@LukasGentele LukasGentele commented Nov 7, 2019

@mgruener That sounds like a smart way of allowing both. Should be straight forward to implement: if len(args) == 0 { run command in shell } else { run command using args (no shell interpretation) }

@FabianKramm What do you think?

@FabianKramm
Copy link
Member

@FabianKramm FabianKramm commented Nov 11, 2019

Yes sounds good to me

@shaunc
Copy link
Contributor

@shaunc shaunc commented Feb 5, 2020

As a variant of what @LukasGentele suggests, it could be done using docker's convention: if command is a list, then interpret as binary w/ arguments; if it is a string, then treat it as something to be run in a shell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.