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

progress: support rendering trackers that haven't started yet #270

Merged
merged 1 commit into from
Aug 12, 2023

Conversation

NathanBaulch
Copy link
Contributor

Proposed Changes

Support displaying trackers that haven't started yet.

I have a complex process that includes tasks that depend on the completion of other tasks. I'd like to display trackers for these deferred tasks without actually starting the clock or animating the indeterminate ones.

This PR has:

  • no breaking changes
  • no race conditions (AFAIK)
  • full unit test coverage
  • demo options to see it in action

@coveralls
Copy link

coveralls commented Aug 12, 2023

Pull Request Test Coverage Report for Build 5839435394

  • 29 of 29 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 5596256371: 0.0%
Covered Lines: 3318
Relevant Lines: 3318

💛 - Coveralls

Copy link
Owner

@jedib0t jedib0t left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. I should be able to merge within a day or two after you address the one comment.

Is it possible to share a demo gif of this in action?

@@ -25,6 +25,8 @@ type Tracker struct {
// Units defines the type of the "value" being tracked
Units Units

DeferStart bool
Copy link
Owner

Choose a reason for hiding this comment

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

Can you move it below the Message variable and add a comment on how this is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@sonarcloud
Copy link

sonarcloud bot commented Aug 12, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@NathanBaulch
Copy link
Contributor Author

Is it possible to share a demo gif of this in action?

I guess so, though it's pretty easy to see in action in demo-progress via the new -rnd-defer flag. This causes random tasks to sit dormant for 3 seconds before commencing.

@jedib0t
Copy link
Owner

jedib0t commented Aug 12, 2023

Is it possible to share a demo gif of this in action?

I guess so, though it's pretty easy to see in action in demo-progress via the new -rnd-defer flag. This causes random tasks to sit dormant for 3 seconds before commencing.

Reason I asked was I'm doing this on mobile and won't have access to a computer for a couple of days. It's all right. I'll check before I merge then.

@NathanBaulch
Copy link
Contributor Author

OK, I gave asciinema a try for the first time, pretty neat!
This shows the progress demo with each task randomly delayed by up to 3 seconds.
https://asciinema.org/a/UztgaKJ9Ld0GEa0TLflXPUTQa

@NathanBaulch
Copy link
Contributor Author

NathanBaulch commented Aug 12, 2023

You'll notice there's a brief period between starting a tracker and the first value increment that looks a little odd IMO.

progress-oddity

This can be fixed relatively easily by adding a secondary timeStart sort in the renderer.

@jedib0t jedib0t merged commit 4ba68d2 into jedib0t:main Aug 12, 2023
3 checks passed
@NathanBaulch
Copy link
Contributor Author

Ha, you merged it just minutes before I pushed a fix for the sorting suggestion above :)

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