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

don't skip compose used as project name #10657

Merged
merged 1 commit into from Jun 7, 2023

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Jun 5, 2023

What I did
fix support for converting docker-compose --project-name compose into a compose v2 command (docker <space> compose)
Tried to understand the fix in #8985 to skip compose argument but doesn't make any sense to me after 18 months. This code is only used if command is docker-compose .. but not when used as a CLI plugin with docker compose ..., maybe this wasn't the case earlier? @ulyssessouza do you remember?

Related issue
fixes #10647

(not mandatory) A picture of a cute animal, if possible in relation to what you did

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof requested review from a team, nicksieger, StefanScherer, ulyssessouza, glours and milas and removed request for a team June 6, 2023 08:25
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Patch coverage: 9.09% and project coverage change: -39.74 ⚠️

Comparison is base (1bd8a77) 59.20% compared to head (60fe974) 19.46%.

Additional details and impacted files
@@             Coverage Diff             @@
##               v2   #10657       +/-   ##
===========================================
- Coverage   59.20%   19.46%   -39.74%     
===========================================
  Files         107      104        -3     
  Lines        9466     9384       -82     
===========================================
- Hits         5604     1827     -3777     
- Misses       3281     7364     +4083     
+ Partials      581      193      -388     
Impacted Files Coverage Δ
cmd/compatibility/convert.go 88.52% <ø> (+3.90%) ⬆️
pkg/compose/build.go 3.88% <0.00%> (-69.18%) ⬇️
pkg/compose/build_buildkit.go 0.00% <0.00%> (-26.54%) ⬇️
pkg/compose/create.go 14.21% <0.00%> (-44.53%) ⬇️
pkg/compose/events.go 0.00% <0.00%> (-79.49%) ⬇️
pkg/compose/start.go 0.00% <0.00%> (-53.77%) ⬇️
pkg/compose/logs.go 39.00% <100.00%> (-0.05%) ⬇️

... and 76 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM from me (although yeah, also curious about that code from the compatibility flag)

Copy link
Member

@milas milas left a comment

Choose a reason for hiding this comment

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

LGTM, I'm guessing this was supposed to be if arg[0] == compose.PluginName originally? But clearly it's not doing anything useful so I am good with eliminating it

@glours glours merged commit e21a8d6 into docker:v2 Jun 7, 2023
22 of 24 checks passed
@ndeloof ndeloof deleted the compose_project_name branch June 7, 2023 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] using "compose" as --project-name makes compose binary error
4 participants