fix #648 #709
fix #648 #709
Conversation
🦋 Changeset detectedLatest commit: 8e31e84 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
|
Oops, i was about to merge and i noticed an issue @khuezy PTAL |
| if (major1 !== major2) return major1 - major2; | ||
| if (minor1 !== minor2) return minor1 - minor2; | ||
| return patch1 - patch2; | ||
| return (patch1 ?? 0) - (patch2 ?? 0); |
There was a problem hiding this comment.
don't you have the same issue with minor?
There was a problem hiding this comment.
Technically we could have this issue for minor2 only.
We only use this to compare with next version, and the next version comes from next/package.json (so should always contain major, minor and patch)
v2 is the one we provide to test, i don't see in which scenario we'd use latest
I should add a comment above the function so that we know about its limitation and how it should be used
There was a problem hiding this comment.
The function assumes that M, m, p exists.
const [major1 = 0, minor1 = 0, patch1 = 0] = v1.split(".").map(Number);
const [major2 = 0, minor2 = 0, patch2 = 0] = v2.split(".").map(Number);
This should default those values.
|
Not sure if important but: if (v1 === "latest") return 1;
// ->
if (v1 === "latest") {
return v2 === "latest" ? 0 : 1;
} |
| * Compare two semver versions. | ||
| * @param v1 - First version. Can be "latest", otherwise it should be a valid semver version in the format of `major.minor.patch`. Usually is the next version from the package.json without canary suffix. If minor or patch are missing, they are considered 0. | ||
| * @param v2 - Second version. Should not be "latest", it should be a valid semver version in the format of `major.minor.patch`. If minor or patch are missing, they are considered 0. | ||
| * @see |
Co-authored-by: Victor Berchet <[email protected]>
I guess we could refactor it later, i'll merge it for now |
3 participants
fix #648