Dockerized Commands in the packaging spec #129
Conversation
| update?: string | DockerizedCommand | ||
| watch?: string | DockerizedCommand |
There was a problem hiding this comment.
If we're going to allow a string array for args in the Docker command, let's do it for native commands too.
| update?: string | DockerizedCommand | |
| watch?: string | DockerizedCommand | |
| update?: string | string[] | DockerizedCommand | |
| watch?: string | string[] | DockerizedCommand |
There was a problem hiding this comment.
Hmm, I'm not sure about that. For native commands, that is not "args", it's a shell command, so it's one string because it ends up as one arg. It's what get passed to (for example) bash -c.
bash -c "echo \"123\"" is not the same as bash -c "echo" "123".
There was a problem hiding this comment.
Correct, I want the capability to provide commands + args directly, not just a shell command. There's no reason to invoke a shell for echo 123, after all, if we can represent it as ["echo", "123"].
There was a problem hiding this comment.
I see. @SandeepSamba do you have the capacity to implement a third methodology here?
There was a problem hiding this comment.
I would prefer it to be just a single string command rather than string[]. We have to invoke the commands via a shell because without a shell, all the fancy(coloured output, formatting,etc) logging that the plugins provide are lost. Even if we provide a string[], the CLI wil concat them with spaces and call it with shell. And with the native packaging RFC we will anyway have to bring in a shell. @SamirTalwar Do you have any particular issue due to the choice of single string vs string[]?
There was a problem hiding this comment.
@SandeepSamba the problem is which shell? Isn't that the point of this. To not use a shell, and just have the CLI run the docker command. Bash isn't normally or always available on windows.
There was a problem hiding this comment.
@codedmart I think @SandeepSamba's point is that he doesn't want to implement string[] (which bypasses the shell) as an alternative to string (which uses the shell), because bypassing the shell stuffs up formatting. The DockerizedCommand is separate to string and string[]. @SamirTalwar's suggestion of adding string[] was in addition to DockerizedCommand.
|
@SandeepSamba is this already implemented? |
No, this will have to to be done once the RFC is approved |
Rendered