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

Rename binary units according to IEC 60027 and add decimal units (SI) #45

Merged
merged 2 commits into from Apr 10, 2022

Conversation

muXxer
Copy link
Contributor

@muXxer muXxer commented Aug 5, 2021

This PR fixes #28

@muXxer
Copy link
Contributor Author

muXxer commented Aug 9, 2021

@vishr maybe you can have a look at this PR? :)
Thank you! :)

@muXxer
Copy link
Contributor Author

muXxer commented Dec 7, 2021

4 months pending? :(

@aldas
Copy link
Contributor

aldas commented Dec 7, 2021

@muXxer it looks good but please change both Format() method signatures back as they were and I will merge this PR.
This is unnecessary breaking change. If you know which PrefixType you want - you can call specific method also.

@muXxer
Copy link
Contributor Author

muXxer commented Mar 1, 2022

Oh I just saw your comment now @aldas 😅

Is it really a breaking change, since the method signatures were extended by a variadic parameter?
If you don't pass the variadic parameter, it defaults back to the former behaviour.

@aldas
Copy link
Contributor

aldas commented Mar 1, 2022

This is unlikely but if someone has made interface for Bytes.Format(b int64) string that previous Bytes struct satisfied with its public method - then this is a breaking change.

@muXxer
Copy link
Contributor Author

muXxer commented Apr 10, 2022

@aldas
Sorry it took so long, almost forgot about that PR :)
Shouldn't be breaking now in terms of public methods.

@codecov-commenter
Copy link

Codecov Report

Merging #45 (ad3d548) into master (4919956) will increase coverage by 1.37%.
The diff coverage is 92.22%.

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
+ Coverage   59.26%   60.64%   +1.37%     
==========================================
  Files           6        6              
  Lines         518      625     +107     
==========================================
+ Hits          307      379      +72     
- Misses        208      242      +34     
- Partials        3        4       +1     
Impacted Files Coverage Δ
bytes/bytes.go 88.97% <92.22%> (-7.52%) ⬇️
color/color.go 88.00% <0.00%> (-0.36%) ⬇️
email/email.go 0.00% <0.00%> (ø)
random/random.go 100.00% <0.00%> (ø)
log/log.go 52.94% <0.00%> (+1.99%) ⬆️

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 4919956...ad3d548. Read the comment docs.

@aldas aldas merged commit 64116ba into labstack:master Apr 10, 2022
@aldas
Copy link
Contributor

aldas commented Apr 10, 2022

better late than never

@muXxer muXxer deleted the feat/add-si-units branch April 11, 2022 08:28
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.

bytes package is not ISO/IEC 80000 and IEEE 1541 complient
3 participants