refactor: allow all semantic versions #18220
Conversation
| assertThat(Version.from("1.0.0-rc2")).isLessThan(Version.from("1.0.0-rc2.1")); | ||
| assertThat(Version.from("1.0.0-rc1")).isGreaterThan(Version.from("1.0.0-beta1")); | ||
| assertThat(Version.from("2.0.0-beta1")).isGreaterThan(Version.from("1.0.0")); | ||
| assertThat(Version.from("1.0.0-alpha1")).isGreaterThan(Version.from("1.0.0-SNAPSHOT")); |
There was a problem hiding this comment.
❗ This is counter intuitive for us, but the semantic version spec doesn't really treat snapshot differently. So right now, I would expect 1.0.0-SNAPSHOT to be the "greatest" version always, such that 1.0.0-alpha1 < 1.0.0-SNAPSHOT < 1.0.0, but this is currently not the case (nor specified in the spec). It's a bit of a gotcha, so maybe @oleschoenburg has some opinion here - but I suspect they simply followed the spec 🤷
Note that previously this was reversed. But since we don't really use the Atomix version in general, it doesn't really matter.
There was a problem hiding this comment.
Can we have a test with the case -alpha1-rc1
There was a problem hiding this comment.
💭 It looks likes a lot of changes, but I actually just use an IntelliJ extension which converts AssertJ stuff, so it was pretty much automated :) Highly recommend it
| checkArgument(major >= 0, "major version must be >= 0"); | ||
| checkArgument(minor >= 0, "minor version must be >= 0"); | ||
| checkArgument(patch >= 0, "patch version must be >= 0"); | ||
| private final String metadata; |
There was a problem hiding this comment.
💭 Backwards compatibility is preserved, since we only add a field, which Kryo can deal with - and that field can be null, it's fine.
There was a problem hiding this comment.
It is only an issue if an old broker receives a version from an updated broker right? So via topology? 🤔
|
Can't backport prior to 8.5 since we don't have |
There was a problem hiding this comment.
Thanks for the quick fix @npepinpe
I added a small regression test to verify that alpha release candidates are allowed.
Before it fails with:
java.lang.IllegalArgumentException: alpha1-rc1 is not a valid build version string
at io.atomix.utils.Version$Build.from(Version.java:179)
at io.atomix.utils.Version.<init>(Version.java:41)
at io.atomix.utils.Version.from(Version.java:70)
at io.atomix.utils.VersionTest.shouldAllowAlphaReleaseCandidates(VersionTest.java:34)
With your changes it succeeds 🚀 Cool stuff 🙇🏼
Modifies the Atomix utility `Version` to use the more complete `SemanticVersion` utility from Zeebe. This fully complies with the semantic version spec, and is much more lenient about adding build-metadata and so on. At the same time, we preserve the check on start up that the version is parse-able at all, to avoid issues where we would run with un-parseable versions and potentially use a snapshot which could cause inconsistencies.
|
I had to rebase the branch to main, as it seem we have a new required check for the merge queue. |
## Description Simple cherry-pick of #18220 > Description Modifies the Atomix utili > ty Version to use the more complete SemanticVersion utility from Zeebe. This fully complies with the semantic version spec, and is much more lenient about adding build-metadata and so on. > > At the same time, we preserve the check on start up that the version is parse-able at all, to avoid issues where we would run with un-parseable versions and potentially use a snapshot which could cause inconsistencies. > ## Related issues Backport #18220 > Related issues > related to https://camunda.slack.com/archives/C03NFMH4KC6/p1714730721838389
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
Successfully created backport PR for |
Zelldon
added
the
version:8.5.1
Description
Modifies the Atomix utility
Versionto use the more completeSemanticVersionutility from Zeebe. This fully complies with the semantic version spec, and is much more lenient about adding build-metadata and so on.At the same time, we preserve the check on start up that the version is parse-able at all, to avoid issues where we would run with un-parseable versions and potentially use a snapshot which could cause inconsistencies.
Related issues
related to https://camunda.slack.com/archives/C03NFMH4KC6/p1714730721838389