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

Strip channel when calling semver.satisfies to allow custom channels #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fasterthanlime
Copy link
Contributor

The semver packages doesn't seem to have any problems dealing with custom channels when calling gt:

> semver.gt('v0.14.1-canary', 'v0.14.0-canary')
true

But satisfies doesn't seem to support them:

> semver.satisfies('v0.14.1-canary', '>=v0.14.0-canary')
false

As a result, nuts always returns HTTP 204 for URLs like:

curl "http://nuts-canary.itch.ovh/update/darwin_x64/0.14.0-canary"

This patch strips the channel before calling semver.satisfies, which fixes the issue for us.

@SamyPesse
Copy link
Member

Sorry for the delay.

I'm not sure it's a "good" idea. I understand the problem (I'm having it with semver and gitbook-cli/gitbook).

But here are the expectations: for our application:

// We want to ship the next beta (or normal)  to user having the beta
satisfies('>=5.0.0-beta.0', '5.0.0-beta.1') == true
satisfies('>=5.0.0-beta.0', '5.0.1') == true

// We don't want to ship the beta to user using the normal version
satisfies('>=4.9.0', '5.0.0-beta.1') == false

I think that your PR will break this behaviour, since:

satisfies('>=4.9.0', '5.0.0-beta.1') -> satisfies('>=4.9.0', '5.0.0') -> true

@fasterthanlime
Copy link
Contributor Author

I agree that it might not be a good idea for everyone.

Personally, I've set up 'unstable' release in a completely different github repo, with a different nuts instance, everything different — I wanted it to be invisible to most users & have the deploy process not touch the main repo at all.

Both approaches have drawbacks, tbh. Do you think there's a way to support both with an option without making the code too complex/costly to maintain? It is a very small patch but as a maintainer I'd completely understand if you'd rather not support it in your version.

@chrisbarless
Copy link
Contributor

chrisbarless commented May 3, 2016

Some more clarity on the matter

https://github.com/npm/node-semver#prerelease-tags

Are there any plans to be able to opt-in for updates to very early channels like Alpha? It would be useful to be able to push updates to QA as they come.

@loprima-l
Copy link

Hi, I merged the project to a new repo to start maintain it, I would be glad if you can put your pull request here : https://github.com/loprima-l/nuts-2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants