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

.: bump go version to go1.21 #304

Closed

Conversation

MadhavJivrajani
Copy link
Contributor

@MadhavJivrajani MadhavJivrajani commented Mar 5, 2024

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

go1.20 is out of support as of ~3 weeks ago and all supported k/k release branches are on go1.21.
This PR bumps version of Go to 1.21.

This PR also cleans up the build tag directives and consistently uses only //go:build now

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:
@dims @liggitt - I'm wondering if we put in go1.21.x in the go.mod or an overall release version like go1.21 should do like we do in k/k?

Release note:

NONE

/assign @dims @liggitt

Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 5, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: MadhavJivrajani
Once this PR has been reviewed and has the lgtm label, please assign deads2k for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 5, 2024
@MadhavJivrajani
Copy link
Contributor Author

@logicalhan FYI ^
for your work on the set implementations using built in constraints rather than defining our own.

@dims
Copy link
Member

dims commented Mar 5, 2024

cc @xdu31

@@ -1,6 +1,6 @@
module k8s.io/utils

go 1.18
go 1.21
Copy link
Member

Choose a reason for hiding this comment

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

/hold

This doesn't do what you think it does :-/

This forces anything importing this module to also bump its own go module directive to go1.21, and makes go change the defaults for godebug compatibility switch defaults. golang/go#65573 is an open proposal to improve this in go1.23, but until that lands, we need to avoid bumping our go directives past go1.20 in our release-from-HEAD repos like utils/kube-openapi/gengo/etc.

Because this repo only releases from HEAD, if we find a bug we need to fix and backport to older kubernetes versions (like 1.27), this would bump the go.mod version in 1.27 from go1.20 to go1.21, which we don't want. Repos that have release branches (like k/k) are more reasonable to keep pushing the go.mod to latest go at HEAD.

golang.org/x/... modules also keep their go directive at the oldest language version you can build with (e.g. https://cs.opensource.google/go/x/sys/+/master:go.mod has 1.18).

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 5, 2024
@liggitt
Copy link
Member

liggitt commented Mar 5, 2024

all supported k/k release branches are on go1.21.

they build with go1.21 now, but 1.27 and 1.28 are relying on go1.20-compatible behavior via their go directive in their go.mod file, which this would force higher if we ever had to pick up a fix from this repo in those branches.

@MadhavJivrajani
Copy link
Contributor Author

Ahhh right
I see what you mean, the decoupling godebug switches proposal that's ongoing. Makes sense.

We can revisit this once we know what happens there, thanks @liggitt!
/close

@k8s-ci-robot
Copy link
Contributor

@MadhavJivrajani: Closed this PR.

In response to this:

Ahhh right
I see what you mean, the decoupling godebug switches proposal that's ongoing. Makes sense.

We can revisit this once we know what happens there, thanks @liggitt!
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants