Replace restic.Progress with new progress.Counter (fixes two race conditions)#3058
Replace restic.Progress with new progress.Counter (fixes two race conditions)#3058MichaelEischer merged 2 commits intorestic:masterfrom
Conversation
1a8ac56 to
5d6426d
Compare
MichaelEischer
left a comment
There was a problem hiding this comment.
Really nice cleanup! I've just a small remark:
I'd like to keep the progress bar creation and the calls to Done() together on the same level to consistently let the progress bar starter stop it. Besides that the code changes are fine.
|
Moved Done() to the call sites of newProgressMax. Index.Load did not have a call site outside of tests. Also removed the Counter argument from checker.ReadData, since that was always called with a nil Counter. |
|
Btw., is it ok if I change the default FPS from 60 to, say, 30, or even 20? It's not like we're trying to create a fluent animation here. |
cfab254 to
6f78747
Compare
This fixes two race conditions while cleaning up the code.
MichaelEischer
left a comment
There was a problem hiding this comment.
LGTM. Thanks for cleaning this up. I'd say we just stick with the 60 fps for now. After all it shouldn't cost too much performance.
This reverts to the old behavior of not printing progress updates on non-interactive terminals. It was accidentally changed in restic#3058.
This reverts to the old behavior of not printing progress updates on non-interactive terminals. It was accidentally changed in restic#3058.
This reverts to the old behavior of not printing progress updates on non-interactive terminals. It was accidentally changed in restic#3058.
What does this PR change? What problem does it solve?
This PR replaces restic.Progress with a new type, ui/progress.Counter. It fixes several issues:
Instead of fixing these issues one by one, I've decided to write a replacement from scratch. It's >100 lines shorter, leaving ample space for a unit test that covers 92% of its statements.
Was the change discussed in an issue or in the forum before?
No.
Checklist
changelog/unreleased/that describes the changes for our users (template here)gofmton the code in all commits