Skip to content

Implement progress indicator#33

Open
Baryonics wants to merge 17 commits into
YanzhaoW:masterfrom
Baryonics:implement_progress_indicator
Open

Implement progress indicator#33
Baryonics wants to merge 17 commits into
YanzhaoW:masterfrom
Baryonics:implement_progress_indicator

Conversation

@Baryonics

@Baryonics Baryonics commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Adding p-ranav/indicators library to this project, to indicate the reader status. (and any other range object)

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Baryonics

Copy link
Copy Markdown
Contributor Author

The indicator feature is functional! I'll need to do some error handling, not quite sure how to do that to be honest... Probably some static assert, since it's almost all template stuff. plus I'm unsure of how to do the testing part. I added the indicator in the reader integration test and it's all covered, but I think it might be necessary to implement unit tests as well.
Whats your opinion @YanzhaoW ?

@YanzhaoW

YanzhaoW commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Thanks for the PR. I will check the code in detail later.

The indicator feature is functional! I'll need to do some error handling, not quite sure how to do that to be honest... Probably some static assert, since it's almost all template stuff.

Yeah, static checking should always be preferred.

plus I'm unsure of how to do the testing part. I added the indicator in the reader integration test and it's all covered, but I think it might be necessary to implement unit tests as well.

I would say an integration test should suffice.

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