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
ints: stricter numbers #4939
ints: stricter numbers #4939
Conversation
* proto: change int32 to uint32 - address comments post merge from: #4332 proto goes down to int32/uint32 it does not provide a smaller size * add contract to comment * change to assertValues * make funcs take uint32 * fix linting * change equal to equalValues
👋 Thanks for creating a PR! Before we can merge this PR, please make sure that all the following items have been
Thank you for your contribution to Tendermint! 🚀 |
* ints: change some ints to int32 - this is to help reduce the overall size of some proto prs. Signed-off-by: Marko Baricevic <marbar3778@yahoo.com> * add safemath for int32 * add safemath for subtraction * add go doc to safemath funcs * move panics to safe math, add checkconvert func * make safe convert accept int64
Codecov Report
@@ Coverage Diff @@
## master #4939 +/- ##
==========================================
+ Coverage 63.34% 63.39% +0.04%
==========================================
Files 189 189
Lines 19585 19584 -1
==========================================
+ Hits 12406 12415 +9
+ Misses 6192 6183 -9
+ Partials 987 986 -1
|
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.
LGTM! Have there been discussions on changing to uints? Many of these should probably never be negative.
there hasnt been any large discussions on when and how to tackle this but it is a want. I think tendermint is one of if not the only one heavily relying on ints. |
ref #2684