Skip to content
This repository was archived by the owner on Nov 27, 2023. It is now read-only.

WIP - Missing e2e - Print container name when successful #85

Merged
merged 1 commit into from
May 13, 2020
Merged

WIP - Missing e2e - Print container name when successful #85

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion cli/cmd/run/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ package run

import (
"context"
"fmt"
"strings"

"github.com/docker/docker/pkg/namesgenerator"
Expand All @@ -45,7 +46,11 @@ func Command() *cobra.Command {
Short: "Run a container",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
return runRun(cmd.Context(), args[0], opts)
if err := runRun(cmd.Context(), args[0], opts); err != nil {
return err
}
fmt.Println(opts.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

question: why not add this code to runRun but here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's just a shame that there is no unity in the commands

return nil
},
}

Expand Down