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
Generate Bindata iff TAGS="bindata" and not up-to-date #10004
Generate Bindata iff TAGS="bindata" and not up-to-date #10004
Conversation
zeripath
commented
Jan 26, 2020
•
edited
edited
- Ensure build tags are passed to go generate
- Move bindata go:generate to files that require build bindata
- Remove duplicate main.go and move to single script in scripts
- Add hash of fileinfo checking to bindata files
This should not be skip-changelog as the build command is changed - and users who use go generate instead of make may need to change how they generate. |
Codecov Report
@@ Coverage Diff @@
## master #10004 +/- ##
==========================================
+ Coverage 42.26% 42.27% +<.01%
==========================================
Files 610 610
Lines 80372 80372
==========================================
+ Hits 33966 33974 +8
+ Misses 42228 42220 -8
Partials 4178 4178
Continue to review full report at Codecov.
|
without
with patch
|
where is the benchmark CI task ;) |
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.
nice refactor ... the time imprufement is not that mouch for me ...
@zeripath can you make needsUpdate to hash files in paralel ? |
The order is important in SHA hashes. I guess we could generate a list and sort it at the end? If you want to see the biggest improvement repeatedly build without tags=bindata as currently we enforce building it even if you don't want it. There's another side effect here - some small machines have problems building bindata so without the earliest part of this pr they won't be able to build. |
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.
Sorry I couldn't see this earlier.
Just a couple of nits, but otherwise LGTM.
|
||
hasher := sha1.New() | ||
|
||
err = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { |
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.
So, we're basically doing make's job. 😆
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.
Yup. That's what you get when you mix two competing build systems ...
Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com>