Include commits since last release in RPM build number#7896
Include commits since last release in RPM build number#7896ocket8888 merged 5 commits intoapache:masterfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7896 +/- ##
=============================================
+ Coverage 31.80% 65.65% +33.84%
Complexity 98 98
=============================================
Files 719 323 -396
Lines 82819 12836 -69983
Branches 970 970
=============================================
- Hits 26340 8427 -17913
+ Misses 54320 4050 -50270
+ Partials 2159 359 -1800
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
8e0fc78 to
c9a6e4f
Compare
Previously, the number of commits ever was used, to ensure that a build from a later child commit will always be considered a higher version number. Because the version number increases each time the release version increases, counting only the commits since the last release tag is sufficient.
c9a6e4f to
650fd46
Compare
|
It looks like either the Makefile changes aren't matching up with the changes elsewhere for some reason, or possibly there's a place where it needs to be changed that's currently being missed. Alternatively, RPM package archive files are totally capable of containing this information - does that really need to be in the filename, too? I know I've said this before, but it seems very redundant to me. |
Yes so that RPMs are sorted in directories, first by version, then by release number.
Yes it is, but this PR isn't meant to change that. That's probably a bigger conversation that might get pushback from some people. |
|
I checked the build artifacts for the checks on this branch - specifically Traffic Ops - and found that it produced
so it looks like that's saying there have been zero commits since 8.1.0. That's incorrect not only because 8.1.0 doesn't technically exist (:P) but because this PR consists of a non-zero amount of commits. tl;dr we got a number now, but it's not the right number. |
That's the version in VERSION
The GitHub Actions only clone the repo with a |
|
So was that always broken? Do we care if the build artifacts match what a user would build? |
Yes it was always broken, RPMs from CI outside this PR say include
nah |
rimashah25
left a comment
There was a problem hiding this comment.
The changes and explanation LGTM.
* Use number of commits since last release in build number Previously, the number of commits ever was used, to ensure that a build from a later child commit will always be considered a higher version number. Because the version number increases each time the release version increases, counting only the commits since the last release tag is sufficient. * Set pipefail and escape $() in Makefile * Include --long to always get relative commit count * Use pipefail shell option within bash * Fall back to commit count of 0 in TPv2 (cherry picked from commit 21d7c9a)
This PR changes the build number to include the number of commits since the last release, rather than the total number of commits ever. Previously, the total commit count was used to ensure that a build from a later child commit will always be considered a higher version number.
But because the version number increases each time the release version increases, counting only the commits since the last release tag is sufficient.
Which Traffic Control components are affected by this PR?
What is the best way to verify this PR?
/aboutTO API endpointPR submission checklist