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

Add known branch exceptions to version matching #242

Merged
merged 1 commit into from Jan 17, 2024

Conversation

paulo-ferraz-oliveira
Copy link
Collaborator

@paulo-ferraz-oliveira paulo-ferraz-oliveira commented Jan 17, 2024

Description

Maybe doing it this way is better, since we don't "break" existing consumption elements, while just adding to already existing exception -rc-.

Forcing version-type: strict, while explicit, would probably force some installations to use a too-specific Elixir version, so we "fix" what we had to prevent that.

Closes #241.

@paulo-ferraz-oliveira
Copy link
Collaborator Author

@josevalim, @grzuy, this adds an exception (to loose version matching) for "known" branches main, maint, and master. Would you be so kind as to check the tests and maybe run this branch where it's failing now, so we can move to merge it? Thanks.

@@ -10292,6 +10292,10 @@ function isRC(ver) {
return ver.match(xyzAbcVersion('^', '(?:-rc\\.?\\d+)'))
}

function isKnownBranch(ver) {
return ['main', 'master', 'maint'].includes(ver)

Choose a reason for hiding this comment

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

If you want to be safer, you could match "maint" anywhere in the version, as OTP has maint-26, maint-25, and so on. :) Just saying, your call, we don't use it right now.

@josevalim
Copy link

Looks great, thank you!

Maybe doing it this way is better, since we don't "break"
existing consumption elements

Forcing `version-type: strict`, while explicit, would
probably force some installations to use a too-specific
Elixir version
Copy link
Member

@starbelly starbelly left a comment

Choose a reason for hiding this comment

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

LGTM!

@paulo-ferraz-oliveira paulo-ferraz-oliveira merged commit b60baf4 into main Jan 17, 2024
60 checks passed
@paulo-ferraz-oliveira paulo-ferraz-oliveira deleted the fix/version-matching-expectations branch January 17, 2024 23:37
@paulo-ferraz-oliveira
Copy link
Collaborator Author

Perfect stuff. Thanks @starbelly. I'll release a patch now.

@paulo-ferraz-oliveira
Copy link
Collaborator Author

v1, v1.17 and v1.17.1 released, pointing to the latest commit.

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.

None yet

3 participants