-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 golang version to 1.19 #1986
✨ Bump golang version to 1.19 #1986
Conversation
Welcome @laxmikantbpandhare! |
Hi @laxmikantbpandhare. 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/test-infra repository. |
I think we shouldn't bump the version. If I'm correct this would force all consumers of CR to use Go 1.19 as well. As long as CR itself doesn't need Go 1.18/19 features it would be probably better to keep 1.17. |
Hm looks like k/k uses Go 1.19 now: https://github.com/kubernetes/kubernetes/blob/v1.25.0/go.mod#L9 |
I checked with @alvaroaleman and decided to follow the suit as it is updated k/k upstream |
/lgtm |
I think we need to bump golangci-lint to a version with go 1.19 similiar to #1976 That I think might also entail some go fmt changes |
Strictly speaking the above is orthogonal to this change and caused by bumping the go version in the presubmit, but we won't be able to merge anything until we do it 🙃 |
Yup definitely |
@alvaroaleman @laxmikantbpandhare Is there already someone working on the golangci-lint bump? Otherwise I can help out. |
@sbueringer - I haven't started yet. If you want you can update. |
On it |
/ok-to-test |
Fix PR: #1987 |
/retest |
1 similar comment
/retest |
The |
/retest |
Looks like the retest worked. @alvaroaleman @sbueringer - Could you please take a look and approve the changes. |
@laxmikantbpandhare there is a merge conflict now, can you please rebase? |
31c5d9d
to
c5c4a41
Compare
@alvaroaleman Did the rebase. |
/lgtm Thx! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, laxmikantbpandhare 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 |
IIRC upstream kubernetes/kubernetes updated the golang version to 1.19 and we are following the same.
It will update the go lang version to 1.19
I already Updated
test-infra
jobs forcontroller-runtime
andcontroller-tools
to 1.19 PR is here: kubernetes/test-infra#27258