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

use loose semver parsing to accept more prerelease formats #125

Merged
merged 2 commits into from
Sep 26, 2021

Conversation

minrk
Copy link
Member

@minrk minrk commented Sep 24, 2021

accepts standards like 1.2.3b4 as seen in Python, etc.

seems like the shortest path to jupyterhub/jupyterhub#3612

@consideRatio
Copy link
Member

Can you add a test case to the existing tests, verifying that 2.0.0b1 should return ["2.0.0b1"] rather than for example ["2", "2.0", "2.0.0", "2.0.0b1"] ?

Nice find with the loose parameter! This looks great to me if it works as intended!

@consideRatio consideRatio added the enhancement New feature or request label Sep 24, 2021
@consideRatio
Copy link
Member

For reference, this is where I read about loose: https://github.com/npm/node-semver#functions

@consideRatio
Copy link
Member

❤️ @manics having written tests

@manics
Copy link
Member

manics commented Sep 24, 2021

I agree a test would be good. Should be easy to add, just copy and paste one of the existing cases e.g.

test("Unsupported prerelease tag", async () => {
const tags = await calculateTags({
token: "TOKEN",
owner: "owner",
repo: "repo",
ref: "refs/tags/2.0.0-rc1",
});
expect(tags).toEqual(["2.0.0-rc1"]);
});

or
test("Handling build number comparisons numerically", async () => {
const scope = nock("https://api.github.com")
.get("/repos/owner/repo/tags")
.reply(200, [
{
name: "1.0.0",
},
{
name: "2.0.0-2",
},
{
name: "1.1.0-2",
},
]);
const tags = await calculateTags({
token: "TOKEN",
owner: "owner",
repo: "repo",
ref: "refs/tags/1.1.0-10",
});
expect(tags).toEqual(["1.1.0-10", "1.1.0", "1.1", "1"]);
scope.done();
});

Assuming it all works, shall we release as 2.0.0 since this is a change in behaviour? Does anyone have any other issues or annoyances involving a breaking change that they want to get into 2.0.0?

accepts standards like 1.2.3b4 as seen in Python, etc.
@minrk
Copy link
Member Author

minrk commented Sep 24, 2021

I only saw 'test.js', which I guess was leftover from an older clone, and not even in the repo! I missed __tests__. Test added (and it works now, thanks to the test!)

@consideRatio
Copy link
Member

LGTM! Thanks @minrk!

@consideRatio consideRatio merged commit 6f187f2 into jupyterhub:main Sep 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants