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

Replace Masterminds/semver with blang/semver #22

Closed
stefanprodan opened this issue Aug 11, 2020 · 6 comments
Closed

Replace Masterminds/semver with blang/semver #22

stefanprodan opened this issue Aug 11, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@stefanprodan
Copy link
Member

stefanprodan commented Aug 11, 2020

The Masterminds semver is seriously flawed see fluxcd/flux#2729
In source controller we decided to use blang/semver for Git tags semver ranges.

@stefanprodan stefanprodan added the enhancement New feature or request label Aug 11, 2020
@bigkevmcd
Copy link
Contributor

I've had a look at this and I'm not sure that it would be an improvement https://play.golang.org/p/2ofl4YOXwdn

I don't think 1.1.0-alpha should be included in 1.0.x?

@squaremo
Copy link
Member

I don't think 1.1.0-alpha should be included in 1.0.x?

Me neither. The spec doesn't talk about ranges or wildcards, but the path of least surprise would surely be to ignore prereleases unless allowed in the pattern; and in particular to ignore them given a wildcard.

Masterminds/semver/v3 has that behaviour for ranges: https://play.golang.org/p/U-v_bYmAiHQ. It also (now?) has a strict parsing mode that will error if the supplied string doesn't conform to the spec https://pkg.go.dev/github.com/Masterminds/semver/v3?tab=doc#StrictNewVersion (NB a leading v is invalid so would have to be stripped before use).

@bigkevmcd
Copy link
Contributor

I agree, but one thing in favour of this change, is that it would be consistent with the https://github.com/fluxcd/source-controller

Probably better to be consistently doing the same (arguably wrong) thing?

It feels like there needs to be consensus on the desired behaviour, and whatever changes applied to get something consistent across the tooling?

@squaremo
Copy link
Member

squaremo commented Sep 2, 2020

I posted fluxcd/source-controller#127 about including prereleases in ranges -- this is seriously broken behaviour.

@dholbach
Copy link
Member

dholbach commented Sep 2, 2020

Hi @bigkevmcd - great to see you here. :-)

@squaremo
Copy link
Member

squaremo commented Nov 18, 2020

Superseded by #38 .

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

No branches or pull requests

4 participants