Skip to content

Fix status bar updates getting lost#389

Merged
mrnugget merged 2 commits into
mainfrom
mrn/fix-stupid-bug
Nov 19, 2020
Merged

Fix status bar updates getting lost#389
mrnugget merged 2 commits into
mainfrom
mrn/fix-stupid-bug

Conversation

@mrnugget

Copy link
Copy Markdown
Contributor

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

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

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

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

mrnugget commented Nov 20, 2020

Copy link
Copy Markdown
Contributor Author

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