theckman / yacspin

Yet Another CLi Spinner; providing over 80 easy to use and customizable terminal spinners for multiple OSes
Apache License 2.0
434 stars 13 forks source link

Remove usage of sync/atomic.Value in favor of mutex #8

Closed theckman closed 4 years ago

theckman commented 4 years ago

Atomic values made sense when the original implementation was simpler, but as it grew the addtional features were wedged-in even when atomic values no longer made sense. This was because the values were related to other fields on the struct so they couldn't be truly atomic.

Let's instead use a mutex so spinner paints are consistent, and so the implementationn is simpler.

codecov[bot] commented 4 years ago

Codecov Report

Merging #8 into master will increase coverage by 3.68%. The diff coverage is 67.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #8      +/-   ##
==========================================
+ Coverage   70.19%   73.88%   +3.68%     
==========================================
  Files           2        2              
  Lines         302      291      -11     
==========================================
+ Hits          212      215       +3     
+ Misses         84       69      -15     
- Partials        6        7       +1     
Impacted Files Coverage Δ
spinner.go 72.75% <67.14%> (+3.79%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4aa3f01...46ed9e6. Read the comment docs.