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

Resume progress bar after timeout #448

Merged
merged 2 commits into from
Apr 26, 2021
Merged

Resume progress bar after timeout #448

merged 2 commits into from
Apr 26, 2021

Conversation

sundowndev
Copy link
Contributor

Q A
πŸ› Bug fix? yes
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #420
❓ Documentation no

Description

The progress bar should be restarted automatically after a timeout if a new event occur. Sometime read actions may be slow but ends with a success after the timeout, for me we should show that something hang bit if it's ends with a success we should resume the spinning.

@sundowndev sundowndev added kind/bug Something isn't working priority/2 labels Apr 19, 2021
@sundowndev sundowndev added this to the v0.8.0 milestone Apr 19, 2021
@sundowndev sundowndev added this to Review in driftctl via automation Apr 19, 2021
@sundowndev sundowndev self-assigned this Apr 19, 2021
@codecov
Copy link

codecov bot commented Apr 19, 2021

Codecov Report

Merging #448 (30bc979) into main (44d1a8e) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #448      +/-   ##
==========================================
+ Coverage   69.71%   69.72%   +0.01%     
==========================================
  Files         286      286              
  Lines        6590     6593       +3     
==========================================
+ Hits         4594     4597       +3     
  Misses       1625     1625              
  Partials      371      371              
Impacted Files Coverage Ξ”
pkg/output/progress.go 95.00% <100.00%> (+0.40%) ⬆️

@sundowndev sundowndev marked this pull request as ready for review April 19, 2021 16:43
@sundowndev sundowndev requested a review from a team as a code owner April 19, 2021 16:43
wbeuil
wbeuil previously approved these changes Apr 20, 2021
Copy link
Contributor

@wbeuil wbeuil left a comment

Choose a reason for hiding this comment

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

Works as expected, good work !

wbeuil
wbeuil previously approved these changes Apr 20, 2021
@sundowndev
Copy link
Contributor Author

sundowndev commented Apr 26, 2021

@wbeuil just rebased this one, can you review again so I can merge it? thanks

Copy link
Contributor

@lotoussa lotoussa left a comment

Choose a reason for hiding this comment

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

LGTM

@sundowndev sundowndev merged commit 9d01944 into main Apr 26, 2021
driftctl automation moved this from Review to Done Apr 26, 2021
@sundowndev sundowndev deleted the fix/progressBarResume branch April 26, 2021 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working priority/2
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants