Skip to content

don't skip compose used as project name#10657

Merged
glours merged 1 commit intodocker:v2from
ndeloof:compose_project_name
Jun 7, 2023
Merged

don't skip compose used as project name#10657
glours merged 1 commit intodocker:v2from
ndeloof:compose_project_name

Conversation

@ndeloof
Copy link
Copy Markdown
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

@ndeloof ndeloof requested review from a team, StefanScherer, glours, milas, nicksieger and ulyssessouza and removed request for a team June 6, 2023 08:25
@ndeloof ndeloof force-pushed the compose_project_name branch from be3a4ef to 60fe974 Compare June 6, 2023 08:25
@codecov
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Contributor

@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
@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