-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add binary size bounds checking #1079
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1079 +/- ##
==========================================
+ Coverage 72.83% 72.91% +0.07%
==========================================
Files 33 33
Lines 2474 2481 +7
==========================================
+ Hits 1802 1809 +7
Misses 565 565
Partials 107 107 Continue to review full report at Codecov.
|
Possible future optimization: instead of the raw size, use the diff between main.go with "hello world" and a main.go with urfave/cli |
Co-Authored-By: Sascha Grunert <sgrunert@suse.com>
What type of PR is this?
What this PR does / why we need it:
This PR adds binary size bounds checking, so that we can avoid more people having issues like that one described in #1055.
Which issue(s) this PR fixes:
Fixes #1057
Changes
build.go
tointernal/build.go
Special notes for your reviewer:
Here's parts of this PR that I find interesting:
2.0
,2.1
.the specific values for the min and max sizes are interesting to me, I think there beingI changed the min and max to just be the min and max size diffs on the various platforms0.5MB
between them is a comfortable distance. I would be open to decreasing the min and max to fit the current values more tightly (eg. min 4.7 max 4.9), thoughPotential Future Work
I thought about adding either of these things:
I think they'd be cool to add ✨ but I'm not that invested in adding them personally.
Trivia
People really care about binary sizes! golang/go#6853
Testing
I briefly changed the min and max values to make sure that they are actually exiting with a failure code and displaying the guidance properly.
Release Notes
There's no user-facing changes in this PR ^_^