robur
/
builder-web
5
0
Fork 0
Code Issues 12 Pull Requests 5 Releases Activity
Labels Milestones

fix version numbering for FreeBSD repository, see #114 #115

Merged
reynir merged 2 commits from fix-version into main 3 months ago
Conversation 1 Commits 2 Files Changed 3
hannes commented 3 months ago
Owner
There is no content yet.
hannes added 1 commit 3 months ago
hannes force-pushed fix-version from a2a2ad9ac7 to b0a5a05070 3 months ago
hannes commented 3 months ago
Poster
Owner

This is btw deployed for builds.robur.coop / pkg.robur.coop and I also re-created the package repository for the offending versions. IMHO good to merge. The "check-versions.sh" could be more automated and actually share some code within the package repo scripts, but we can leave it as is for now.

This is btw deployed for builds.robur.coop / pkg.robur.coop and I also re-created the package repository for the offending versions. IMHO good to merge. The "check-versions.sh" could be more automated and actually share some code within the package repo scripts, but we can leave it as is for now.
reynir approved these changes 3 months ago
reynir left a comment

It seems to me that with this change we have a stronger assumption of versions being of format <major>.<minor>.<patch>. I'm not opposed to this, but it's good to be aware of.

packaging/versions.txt Outdated
2.0.0-10-gabcdef-20220202-hahh 2.0.0-11-g123456-20220201-abcd
2.0.0-10-gabcdef-20220202-hahh 2.0.0-110-g123456-20220201-abcd
2.0.0-11-g123456-20220201-abcd 2.0.1-20220120-abcd
3.0-20230101-abcd 3.0.1-20230204-bdbd
reynir commented 3 months ago
Poster
Owner

Do we lose support for version numbers without the patch part? E.g. 3.0-...

Do we lose support for version numbers without the patch part? E.g. 3.0-...
hannes commented 3 months ago
Poster
Owner

Yes we do. Well, and we do not.

So the current state of our package repository is that all packages adhere to the MAJOR.MINOR.PATCH form (or MAJOR.MINOR.PATCH-#COMMITS-g<hash>). This PR adds some magic string (.0.g0000000) when the version is exactly such (figuring out by counting the '.' characters). So when you do tags / releases of a different form (MAJOR.MINOR), that code won't be executed. An alternative strategy could be to check whether "g" is present (though the number of hex chars varies, 6 or 7 are a good guess) -- maybe that is actually more sane? WDYT?

Yes we do. Well, and we do not. So the current state of our package repository is that all packages adhere to the `MAJOR.MINOR.PATCH` form (or `MAJOR.MINOR.PATCH-#COMMITS-g<hash>`). This PR adds some magic string (.0.g0000000) when the version is exactly such (figuring out by counting the '.' characters). So when you do tags / releases of a different form (MAJOR.MINOR), that code won't be executed. An alternative strategy could be to check whether "g<hex><hex><hex>" is present (though the number of hex chars varies, 6 or 7 are a good guess) -- maybe that is actually more sane? WDYT?
reynir commented 3 months ago
Poster
Owner

On one hand I think it is more robust/sane. On the other hand I suspect it's less straightforward code and I am fine with stronger assumption of the major/minor/patch format.

On one hand I think it is more robust/sane. On the other hand I suspect it's less straightforward code and I am fine with stronger assumption of the major/minor/patch format.
hannes commented 3 months ago
Poster
Owner

I force-pushed and use grep for a bunch of hex chars instead of counting dots.

I suspect there could be trouble if versions / tags do not conform to MAJOR.MINOR.PATCH -- maybe there should be some sanitizing and warning instead of just using what is there.

That check is done in c545553e67

Thanks for your review, IMHO this is ready to be merged now :)

I force-pushed and use `grep` for a bunch of hex chars instead of counting dots. I suspect there could be trouble if versions / tags do not conform to MAJOR.MINOR.PATCH -- maybe there should be some sanitizing and warning instead of just using what is there. That check is done in https://git.robur.io/robur/builder-web/commit/c545553e672e6101e7c59133bd0acc2a6af0ff16 Thanks for your review, IMHO this is ready to be merged now :)
hannes force-pushed fix-version from b0a5a05070 to 03c00fe018 3 months ago
hannes added 1 commit 3 months ago
reynir added 1 commit 3 months ago
reynir force-pushed fix-version from 677403a1b6 to 73fbb59377 3 months ago
reynir merged commit 88c91c0856 into main 3 months ago
reynir deleted branch fix-version 3 months ago

Reviewers

reynir approved these changes 3 months ago
The pull request has been merged as 88c91c0856.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
reynir
Labels
Apply labels
Clear labels
No items
No Label
Milestone
Set milestone
Clear milestone
No items
No Milestone
Assignees
Assign users
Clear assignees
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'fix-version'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes