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

progress: minor correctness fixes #10871

Merged
merged 1 commit into from Aug 3, 2023
Merged

Conversation

milas
Copy link
Member

@milas milas commented Aug 3, 2023

What I did

  • When waiting for dependencies, select on the context as well as the ticker
  • Write multiple progress events "transactionally" (i.e. hold the lock for the duration to avoid other events being interleaved)
  • Do not change "finished" steps back to "in progress" to prevent flickering

There are some bigger issues here I want to address, but these are a few quick fixes.

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

@milas milas requested a review from a team August 3, 2023 13:20
@milas milas self-assigned this Aug 3, 2023
@milas milas requested review from nicksieger, ndeloof, StefanScherer, ulyssessouza, glours and laurazard and removed request for a team August 3, 2023 13:20
* When waiting for dependencies, `select` on the context as well
  as the ticker
* Write multiple progress events "transactionally" (i.e. hold the
  lock for the duration to avoid other events being interleaved)
* Do not change "finished" steps back to "in progress" to prevent
  flickering

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Patch coverage: 33.33% and project coverage change: +0.04% 🎉

Comparison is base (8a1bf5d) 58.30% compared to head (4e31bae) 58.35%.
Report is 5 commits behind head on v2.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10871      +/-   ##
==========================================
+ Coverage   58.30%   58.35%   +0.04%     
==========================================
  Files         119      119              
  Lines       10331    10343      +12     
==========================================
+ Hits         6024     6036      +12     
  Misses       3710     3710              
  Partials      597      597              
Files Changed Coverage Δ
pkg/progress/tty.go 39.03% <27.27%> (-0.70%) ⬇️
pkg/compose/convergence.go 69.81% <50.00%> (-0.21%) ⬇️

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@milas milas merged commit 80856ea into docker:v2 Aug 3, 2023
25 checks passed
@milas milas deleted the misc-progress-fixes branch August 3, 2023 19:14
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.

None yet

3 participants