feat: set a better version for dev builds #1415
Conversation
robbkidd
changed the title
robbkidd
added
the
version: no bump
| VERSION="${CIRCLE_BUILD_NUM}" | ||
| # If we are doing a dev build on circle, append the build number (job id) to the version | ||
| if [[ -n "${CIRCLE_BUILD_NUM:-}" ]]; then | ||
| VERSION="${VERSION}-ci${CIRCLE_BUILD_NUM}" |
There was a problem hiding this comment.
| VERSION="${VERSION}-ci${CIRCLE_BUILD_NUM}" | |
| VERSION="${VERSION}-${CIRCLE_BUILD_NUM}" |
So I love the general idea of this. However, I find the full version a bit distracting with the extra descriptors. I also wonder if the 45 commits is necessary. I don't know if it's possible to drop the g, but I feel like it would be clearer if we dropped the g and the ci.
A radical idea could be something like...
v2.8.2-45-ge527ea1-ci8675309 becomes v2.8.2+e527ea1_8675309
There was a problem hiding this comment.
Personally, I don't see the value in the 45. I also like dropping g and ci, but I don't think we win much with the + and _ -- I'd rather see dashes or underscores everywhere.
There was a problem hiding this comment.
I like the idea to quickly know how much deviation between a dev build and the last stable release. Since the CI build ID is always a number, I find it helpful to have the ci prefix to remind me what the number is. However, I agree that the g for git is not as helpful since commit hash also contains letters.
There was a problem hiding this comment.
Starting with defaults, the 45 and the g is what comes out of the git describe command:
❯ git describe --tags --match='v[0-9]*' --always
v2.8.2-45-ge527ea1
gfor git is not as helpful since commit hash also contains letters
I agree that's annoying. I happen to know that commit IDs are SHA1 hashes and SHA1 hashes are hexadecimal and g is not a hex digit, so my brain parses that fine and it doesn't bother me that much. … But as I list all that out, I see that it takes me knowing a lot of things to get to the point of it not bothering me much.
So my current tolerance for this format is such that I am currently not convinced that the added complexity to shape this dev version differently will be worth writing and maintaining. I am open to being convinced, though.
There was a problem hiding this comment.
I think it's not worth fixing that for a minor annoyance. I'm going to approve this, but it might be useful to also hear from others before merging.
There was a problem hiding this comment.
Oh, that's makes much more sense. Thank you for the explanation. My brain can now process it better. Always learning something new from Robb.
I agree with you that this is not something that worth adding more complexity for
There was a problem hiding this comment.
Agreed, not worth the added effort. This gives me so much information so I'm plenty pleased 😃
There was a problem hiding this comment.
I also don't find much value in the ci prefix for circle build IDs. I'm don't mind the hyphen's or the number of commits as they match the default git format.
Also agree there's we've been blocked on this for awhile, we can always come back and change it if needed but this better than what we have now.
| VERSION="${CIRCLE_BUILD_NUM}" | ||
| # If we are doing a dev build on circle, append the build number (job id) to the version | ||
| if [[ -n "${CIRCLE_BUILD_NUM:-}" ]]; then | ||
| VERSION="${VERSION}-ci${CIRCLE_BUILD_NUM}" |
There was a problem hiding this comment.
I think it's not worth fixing that for a minor annoyance. I'm going to approve this, but it might be useful to also hear from others before merging.
|
Added a version number decoder ring to doc comments, as @VinozzZ recommended. |
| VERSION="${CIRCLE_BUILD_NUM}" | ||
| # If we are doing a dev build on circle, append the build number (job id) to the version | ||
| if [[ -n "${CIRCLE_BUILD_NUM:-}" ]]; then | ||
| VERSION="${VERSION}-ci${CIRCLE_BUILD_NUM}" |
There was a problem hiding this comment.
I also don't find much value in the ci prefix for circle build IDs. I'm don't mind the hyphen's or the number of commits as they match the default git format.
Also agree there's we've been blocked on this for awhile, we can always come back and change it if needed but this better than what we have now.
|
Gonna take this as it is now. We can always make more changes later. |
Which problem is this PR solving?
Prior to these changes, dev builds in CI have only an integer set for their version (main.BuildID). That left humans to:
Short description of the changes
What if we encode most of that in the version number?
git describe- Return the most recent annotated tag that is reachable from a commit.--tags- OK, any tag, not just the annotated ones. We don't always remember to annotate a version tag.--match='v[0-9]*'- But of all those tags, only the version ones (starts with a v and a digit).--always- … and if a tag can't be found, fallback to the commit ID.Example of a version computed in CI for any build that isn't a tagging event:
That the version contains more than
v2.8.2indicates:git commite527ea1.45commits ahead of the commit tagged withv2.8.2.8675309.