WIP - Missing e2e - Print container name when successful #85
Conversation
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
| if err := runRun(cmd.Context(), args[0], opts); err != nil { | ||
| return err | ||
| } | ||
| fmt.Println(opts.name) |
There was a problem hiding this comment.
question: why not add this code to runRun but here?
There was a problem hiding this comment.
Not an important thing, but if we look at the runRun it can be used in anything... So I look at it as a clean function. And the output (in this case opt.name) of this is related to the command line which is cobra related code.
So I just prefer to keep the command line specific code together and keep the generic stuff out of this.
And yes, I know the intent of this function is to be used only by Command() but it looks cleaner to me like this.
There was a problem hiding this comment.
I see what you mean, we don't have the same interpretation of this function, for me runRun and all others is there just so that the creation of the command is cleaner and without extra code, only the declarative things go there.
There was a problem hiding this comment.
It's just a shame that there is no unity in the commands
Implements
runpart of https://github.com/docker/api/issues/54This PR depends on the merge of https://github.com/docker/api/pull/76 to enable the test of the output of the
runcommand