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

Fix status bar updates getting lost #389

Merged
merged 2 commits into from
Nov 19, 2020
Merged

Fix status bar updates getting lost #389

merged 2 commits into from
Nov 19, 2020

Conversation

mrnugget
Copy link
Contributor

This is super embarrassing, but ... well, here we are.

@mrnugget mrnugget requested a review from a team November 19, 2020 14:46
Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

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

OMG SO EMBARASSING

LGTM

@mrnugget mrnugget merged commit 348cfec into main Nov 19, 2020
@mrnugget mrnugget deleted the mrn/fix-stupid-bug branch November 19, 2020 15:21
@LawnGnome
Copy link
Contributor

I don't think I understand why this makes a difference. Wasn't numStatusBars still in scope at the point the later check occurred anyway? 😕

@mrnugget
Copy link
Contributor Author

mrnugget commented Nov 20, 2020

The PrintStatuses method is used as a callback and is called every ~second to print the current status.

The first time it's called, the progress bar and the status bars haven't been initialised, so the code behind if p.progress == nil is executed.

The second, third, ..., nth time it is called, the bars have been initialised, which means if p.progress == nil is false and 🐛 the numStatusBars stays 0 🐛 , which then leads to the "which statusbar do I use to show progress for this new worker?"-check to result in "no statusbar left".

@LawnGnome
Copy link
Contributor

The PrintStatuses method is used as a callback and is called every ~second to print the current status.

Aha! That was the bit of context I'd forgotten. Thanks for that!

scjohns pushed a commit that referenced this pull request Apr 24, 2023
* Fix status bar updates getting lost

* Add changelog entry
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.

3 participants