-
Notifications
You must be signed in to change notification settings - Fork 1.6k
✨ (CLI, go/v4): Upgrade Golang version to v1.24.0 #4585
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
✨ (CLI, go/v4): Upgrade Golang version to v1.24.0 #4585
Conversation
Welcome @dongjiang1989! |
Hi @dongjiang1989. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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-sigs/prow repository. |
/ok-to-test |
@camilamacedo86 How to change golang version here? Maybe change prow image |
We will need to update the the test-infra: http://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes-sigs/kubebuilder/kubebuilder-presubmits.yaml#L14 Feel free to push a PR and ping me. |
I saw that you open the PR there as well. |
Just for we keep tracked here: |
Let's bump controller-runtime latest release which still uses 1.23 : https://github.com/kubernetes-sigs/controller-runtime/blob/v0.20.3/go.mod Do a patch release, and then I think we might can move forward with this one. |
Thanks @camilamacedo86 Patch it to controller-runtime v0.20.x? |
/lgtm |
we need to upgrade the golang-ci to allow it pass |
@camilamacedo86 checking errors, maybe we do not need to update linter for this one to pass. I think error is related to how it is being installed. If we use Line 128 in 2b35f33
kubebuilder/testdata/project-v4/Makefile Line 221 in 2b35f33
and in github actions kubebuilder/.github/workflows/lint.yml Line 29 in 2b35f33
I think CI will work for this one (without changing linter version) |
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.
Can you please help us ensure that it pass in the CI and also squash the commits to allow us to move forward?
a927b6d
to
49d5eac
Compare
49d5eac
to
f91e8fb
Compare
Signed-off-by: dongjiang <[email protected]> fix golangci lint Signed-off-by: dongjiang <[email protected]> revent golangcl-lint Signed-off-by: dongjiang <[email protected]> add go install mode Signed-off-by: dongjiang <[email protected]>
f91e8fb
to
1ca4470
Compare
Done :) |
@@ -38,6 +38,7 @@ jobs: | |||
- name: Run linter | |||
uses: golangci/golangci-lint-action@v6 | |||
with: | |||
install-mode: goinstall |
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 understand that allowing us to pass the CI and ensure that the lint works was required.
Am I right? Would it work without this change?
This change solved the issue
Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0)
Failed executing command with error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0)
Right?
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.
Yes. Solved it done.
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.
Lets move forward but I create an issue to see if we can remove it before the next release or if we should leave it as it is now: #4750
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.
This was not the right fix.
You are using golangci-lint v1.63, the first version of golangci-lint that supports go1.24 is golangci-lint v1.64.
The right fix is to update the golangci-lint version to a version that support go1.24
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.
Thanks @ldez
Got it
Thanks @camilamacedo86 Please re-check 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.
/lgtm
/approve
/ok-to-test
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, dongjiang1989 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 |
Upgrade go version 1.23 to 1.24