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 support for patch versions of Bazel (e.g. 1.2.3-patch4) #280

Merged
merged 1 commit into from
Dec 8, 2021
Merged

Add support for patch versions of Bazel (e.g. 1.2.3-patch4) #280

merged 1 commit into from
Dec 8, 2021

Conversation

UebelAndre
Copy link
Contributor

I have some patches I maintain in a fork of Bazel and generate binaries with -patch suffixes followed by a number (e.g. 4.2.1-patch1) and would like to start using Bazelisk to download those binaries. This mostly works already except that Bazelisk does not accept the -patch patch versions. The change is pretty small so thought I'd open a pull requests to learn whether or not this patching pattern was a good idea or if maybe there's a better way. I also understand not everyone may be doing this so am open to other ideas and approaches for using patched Bazel versions.

@philwo philwo requested a review from fweikert November 20, 2021 11:24
Copy link
Member

@philwo philwo left a comment

Choose a reason for hiding this comment

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

LGTM from my side, Florian what do you think?

@UebelAndre
Copy link
Contributor Author

@fweikert Didn't realize there was a merge conflict here. That's been resolved and this PR is ready for a review if you can find the time 🙏

@UebelAndre
Copy link
Contributor Author

@fweikert @philwo Another friendly ping 😅

@fweikert
Copy link
Member

fweikert commented Dec 8, 2021

Sorry, I've been OOO for two weeks. Will take a look later today

@@ -18,6 +18,7 @@ const (

var (
releasePattern = regexp.MustCompile(`^(\d+)\.(x|\d+\.\d+)$`)
patchPattern = regexp.MustCompile(`^(\d+\.\d+\.\d+)-([\w\d]+)$`)
Copy link
Member

Choose a reason for hiding this comment

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

I think we should start with a more restrictive regex and then maybe loosen it later. what about ...-patch\d+
?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restricting it to just -patch would not satisfy my needs unfortunately 😞 Do you feel strongly that it should be restrictive?

Copy link
Member

Choose a reason for hiding this comment

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

No, it's fine as-is.

@fweikert fweikert merged commit 6183822 into bazelbuild:master Dec 8, 2021
@UebelAndre UebelAndre deleted the patches branch December 8, 2021 19:05
@UebelAndre
Copy link
Contributor Author

Thank you so much! Do you know when the next release will be?

@fweikert
Copy link
Member

fweikert commented Dec 8, 2021

We can cut a new one this week.

@fweikert
Copy link
Member

Update: I released v1.11.0 yesterday, including this change.

@UebelAndre
Copy link
Contributor Author

Thank you so much!

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