-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
fix"convert --images" output to right format #9904
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR - good catch!
Left a comment re: the separator, as there is a global for this that gets set based on whether Compose is running in compatibility mode or not.
Thanks for making the change - unfortunately, the new commit is missing DCO sign-off so isn't passing CI. Rebase instructions @ https://github.com/docker/compose/pull/9904/checks?check_run_id=9738031634 |
cmd/compose/convert.go
Outdated
@@ -240,7 +240,7 @@ func runConfigImages(opts convertOptions, services []string) error { | |||
if s.Image != "" { | |||
fmt.Println(s.Image) | |||
} else { | |||
fmt.Printf("%s_%s\n", project.Name, s.Name) | |||
fmt.Printf("%s\n", project.Name+api.Separator+s.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as we already rely on Printf
, better not use +
to append strings
Signed-off-by: windforce17 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Windforce17 [email protected]
What I did
According to docker images name rule: https://docs.docker.com/engine/reference/commandline/tag/#description
docker compose convert --images
should not contain underscores.closes #9920