Skip to content

use --progress to configure progress UI style#10690

Merged
ndeloof merged 1 commit intodocker:v2from
ndeloof:progress
Jun 12, 2023
Merged

use --progress to configure progress UI style#10690
ndeloof merged 1 commit intodocker:v2from
ndeloof:progress

Conversation

@ndeloof
Copy link
Copy Markdown
Contributor

@ndeloof ndeloof commented Jun 12, 2023 •
edited
Loading

What I did
Introduce --progress top-level flag to configure progress UI as auto|tty|plain|quiet|none

This allows to have explicit control on the progress UI without using --ansi flag which also has impact on logs rendering

build already declared this flag. It is now redundant and kept for backward compatibility but marked hidden

Related issue
see #10686

(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, laurazard, milas, nicksieger and ulyssessouza and removed request for a team June 12, 2023 07:38
@ndeloof ndeloof force-pushed the progress branch 2 times, most recently from 8a7ac24 to 1f20407 Compare June 12, 2023 07:49
milas
milas approved these changes Jun 12, 2023
View reviewed changes
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! Hurray for CLI consistency

(I mean I'm not thrilled that progress.Mode is a global but that's not new 😛)

@ndeloof ndeloof force-pushed the progress branch 2 times, most recently from a32915f to 000d3a3 Compare June 12, 2023 13:33
@ndeloof
Copy link
Copy Markdown
Contributor Author

ndeloof commented Jun 12, 2023

@milas made a last minute update : to avoid --progress for being documented on each and every command, I made build --progress a simple duplicate to set the same variable, and marked flag hidden so we keep backward compatibility but docs are relevant for recommended usage.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 12, 2023 •
edited
Loading

Codecov Report

Patch coverage: 36.84% and project coverage change: -0.14 ⚠️

Comparison is base (68bd0eb) 58.98% compared to head (000d3a3) 58.85%.

❗ Current head 000d3a3 differs from pull request most recent head 43665d4. Consider uploading reports for the commit 43665d4 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##               v2   #10690      +/-   ##
==========================================
- Coverage   58.98%   58.85%   -0.14%     
==========================================
  Files         111      112       +1     
  Lines        9716     9740      +24     
==========================================
+ Hits         5731     5732       +1     
- Misses       3399     3419      +20     
- Partials      586      589       +3
Impacted Files Coverage Δ
pkg/progress/quiet.go 0.00% <0.00%> (ø)
pkg/progress/writer.go 61.53% <0.00%> (-2.47%) ⬇️
cmd/compose/compose.go 70.94% <41.66%> (-2.02%) ⬇️
cmd/compose/build.go 81.15% <80.00%> (+6.83%) ⬆️

... and 4 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.

glours
glours approved these changes Jun 12, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

LGTM

@ndeloof ndeloof enabled auto-merge (rebase) June 12, 2023 14:34
@ndeloof ndeloof merged commit cfe91be into docker:v2 Jun 12, 2023
@ndeloof ndeloof deleted the progress branch June 13, 2023 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@glours glours glours approved these changes

@nicksieger nicksieger Awaiting requested review from nicksieger nicksieger was automatically assigned from docker/compose-maintainers

@StefanScherer StefanScherer Awaiting requested review from StefanScherer StefanScherer was automatically assigned from docker/compose-maintainers

@ulyssessouza ulyssessouza Awaiting requested review from ulyssessouza ulyssessouza was automatically assigned from docker/compose-maintainers

@laurazard laurazard Awaiting requested review from laurazard laurazard was automatically assigned from docker/compose-maintainers

+1 more reviewer

@milas milas milas approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ndeloof @glours @milas