Skip to content

refactor: use progress_bar more explicitly as a thread #1622

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

Merged
merged 2 commits into from
Nov 24, 2022

Conversation

aron-bram
Copy link
Collaborator

@aron-bram aron-bram commented Nov 23, 2022

I did not like the way the progress bar class started itself as soon as it was instatiated, so I changed the way it works to be more intuitive. Namely, ProgressBar can now be used just like a regular thread, so start() has to be called on it for it to start running. Also, join is used to wait for it to finish, which at the same time terminates the thread by maxing out the bar, so that the thread will never hang.

I also added improved docs.

@aron-bram aron-bram changed the title refactor: use progress_bar more explicitely as a thread refactor: use progress_bar more explicitly as a thread Nov 23, 2022
@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Merging #1622 (318cfd2) into development (1abd1f9) will decrease coverage by 0.14%.
The diff coverage is 90.00%.

Additional details and impacted files
@@               Coverage Diff               @@
##           development    #1622      +/-   ##
===============================================
- Coverage        83.42%   83.27%   -0.15%     
===============================================
  Files              156      156              
  Lines            11927    11931       +4     
  Branches          1896     1897       +1     
===============================================
- Hits              9950     9936      -14     
- Misses            1412     1425      +13     
- Partials           565      570       +5     

Impacted file tree graph

Copy link
Contributor

@eddiebergman eddiebergman left a comment

Choose a reason for hiding this comment

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

Looks good, I'll leave the documentation part up to you to decide if you want to fix it, otherwise feel free to merge it yourself!

Make sure to use the Squash and Merge option when merging, it should show on the button if not already.

@aron-bram aron-bram merged commit 59ea4b0 into development Nov 24, 2022
@aron-bram aron-bram deleted the progress_bar branch November 24, 2022 12:12
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.

2 participants