-
Notifications
You must be signed in to change notification settings - Fork 360
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
Improvement/import tool progress indication #804
Improvement/import tool progress indication #804
Conversation
Codecov Report
@@ Coverage Diff @@
## master #804 +/- ##
==========================================
+ Coverage 41.88% 41.89% +0.01%
==========================================
Files 133 133
Lines 10422 10435 +13
==========================================
+ Hits 4365 4372 +7
- Misses 5477 5482 +5
- Partials 580 581 +1
Continue to review full report at Codecov.
|
cmdutils/progress.go
Outdated
current *int64 | ||
total *int64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think that it will be better to have these as int64
and when we call the atomic increment get the address of the variable - so we will not be required to handle nil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related to the previous talk about this code -
The doc about why you need to use atomic load when you use atomic API - https://preshing.com/20130618/atomic-vs-non-atomic-operations/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool
Closes #798