Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecation(semver): deprecate eq(), neq(), lt(), lte(), gt() and gte() #4048

Conversation

timreichen
Copy link
Contributor

ref: #3948 (comment)

  • deprecates eq(), neq(), lt(), lte(), gt() and gte()
  • adds common use cases to compare() jsdoc

@timreichen timreichen requested a review from kt3k as a code owner January 2, 2024 00:22
@github-actions github-actions bot added the semver label Jan 2, 2024
@timreichen timreichen mentioned this pull request Jan 2, 2024
13 tasks
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the overall idea of this PR. I have just a few nits.

It'd be good to hear other's opinions on this change.

Comment on lines +58 to +101
// taken over from `eq_test.ts`
Deno.test({
name: "compare()",
fn: async (t) => {
// [version1, version2]
// version1 should be greater than version2
const versions: [string, string, number][] = [
["0.0.0", "0.0.0", 0],
["1.2.3", "1.2.3", 0],
["1.2.3-pre.0", "1.2.3-pre.0", 0],
["1.2.3-pre.0+abc", "1.2.3-pre.0+abc", 0],
["0.0.0", "0.0.0-foo", 1],
["0.0.1", "0.0.0", 1],
["1.0.0", "0.9.9", 1],
["0.10.0", "0.9.0", 1],
["0.99.0", "0.10.0", 1],
["2.0.0", "1.2.3", 1],
["1.2.3", "1.2.3-asdf", 1],
["1.2.3", "1.2.3-4", 1],
["1.2.3", "1.2.3-4-foo", 1],
["1.2.3-5", "1.2.3-5-foo", -1], // numbers > strings, `5-foo` is a string not a number
["1.2.3-5", "1.2.3-4", 1],
["1.2.3-5-foo", "1.2.3-5-Foo", 1],
["3.0.0", "2.7.2+asdf", 1],
["1.2.3-a.10", "1.2.3-a.5", 1],
["1.2.3-a.5", "1.2.3-a.b", -1],
["1.2.3-a.b", "1.2.3-a", 1],
["1.2.3-a.b.c.10.d.5", "1.2.3-a.b.c.5.d.100", 1],
["1.2.3-r2", "1.2.3-r100", 1],
["1.2.3-r100", "1.2.3-R2", 1],
];

for (const [v0, v1, expected] of versions) {
await t.step(`${v0} == ${v1}`, () => {
const s0 = parse(v0);
const s1 = parse(v1);

const eq = compare(s0, s1);
const op = expected ? "==" : "!=";
assertEquals(eq, expected, `${s0} ${op} ${s1}`);
});
}
},
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only test needed. It takes care of all of the other equality and inequality cases more precisely.

Co-authored-by: Asher Gomez <ashersaupingomez@gmail.com>
@kt3k
Copy link
Member

kt3k commented Jan 2, 2024

I'd prefer to keep these methods as they are easier to remember than the meaning of return values of compare

@timreichen
Copy link
Contributor Author

I'd prefer to keep these methods as they are easier to remember than the meaning of return values of compare

Ok, I will close this in favour of another PR.

@timreichen timreichen closed this Jan 4, 2024
@timreichen timreichen deleted the semver_deprecate_compare_helper_functions branch January 7, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants