-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
🌱 Bump Go to 1.21.5 #9900
🌱 Bump Go to 1.21.5 #9900
Conversation
cbde833
to
4d05f95
Compare
/retitle 🌱 Bump Go to 1.21.5 |
@@ -1,6 +1,6 @@ | |||
module sigs.k8s.io/cluster-api | |||
|
|||
go 1.20 | |||
go 1.21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT about also setting toolchain like CR? k/k didn't do it.
After reading https://go.dev/doc/toolchain I'm not sure if we should or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The go and toolchain lines can be thought of as specifying the version requirements for the module's dependency on the Go toolchain itself, just as the require lines in go. mod specify the version requirements for dependencies on other modules.
Based on this it sounds reasonable to set it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting toolchain to go1.21.5 would force folks who import this or newer versions of capi to use go >= 1.21.5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we already do it via go 1.21
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked to Christian about it. We don't see a strong reason why we should set toolchain
. We already enforce usage of Go 1.21 via go 1.21
and we probably don't want to enforce a specific minimum go patch version if we don't have a strong reason to do so.
Thus I would merge as is. If someone comes up with a reason why we should set it, let us know and we follow-up
/lgtm |
LGTM label has been added. Git tree hash: b6d6691ecb04228ded594a2e01cf3111da7030af
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Bumps go to 1.21.5.
Upstream kubernetes did bump at kubernetes/kubernetes#122201
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Part of #9578
/area dependency