-
Notifications
You must be signed in to change notification settings - Fork 284
🐛 upgrade golangci-lint for go 1.18 support #1184
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
Conversation
|
/cherry-pick release-0.5 |
|
/lgtm thanks @apricote 👌🏻 |
seanschneeweiss
left a comment
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
Thank you very much for fixing this.
pkg/record/recorder.go
Outdated
| // Event constructs an event from the given information and puts it in the queue for sending. | ||
| func Event(object runtime.Object, reason, message string) { | ||
| defaultRecorder.Event(object, corev1.EventTypeNormal, strings.Title(reason), message) | ||
| defaultRecorder.Event(object, corev1.EventTypeNormal, cases.Title(language.Und).String(reason), message) |
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.
Out of curiosity, what is the language.Und doing?
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.
same question here..
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 this is for Undefined language, it's not really explained in the [docs]. Using this language tag returned the expected result, as you can see in this Go Playground example: https://go.dev/play/p/fNew-KrewPY
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.
Makes sense. Maybe change it to language.English then?
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 that makes sense, considering the reasons should be in english so all consumers of them can understand them.
|
Unknown CLA label state. Rechecking for CLA labels. Send feedback to sig-contributor-experience at kubernetes/community. /check-cla |
|
/check-cla I think prow had some issues today |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: apricote, tobiasgiese 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 |
|
/cherry-pick release-0.5 |
|
@tobiasgiese: once the present PR merges, I will cherry-pick it on top of release-0.5 in a new PR and assign it to you. In response to this:
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. |
|
/test pull-cluster-api-provider-openstack-test |
|
@apricote guess you forgot to run |
|
|
||
| // OpenStackClusterSpec defines the desired state of OpenStackCluster. | ||
| type OpenStackClusterSpec struct { | ||
|
|
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.
unrelated change..
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.
These are changes that golangci-lint requested after I upgrade it. In this case the structs were not formatted with gofumpt
pkg/record/recorder.go
Outdated
| // Event constructs an event from the given information and puts it in the queue for sending. | ||
| func Event(object runtime.Object, reason, message string) { | ||
| defaultRecorder.Event(object, corev1.EventTypeNormal, strings.Title(reason), message) | ||
| defaultRecorder.Event(object, corev1.EventTypeNormal, cases.Title(language.Und).String(reason), message) |
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.
same question here..
✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Looks like I forgot to run it a second time after fixing the linting issues. New commit is up |
With versions lower than 1.45.0, golangci-lint returns a lot of invalid errors when running in Go 1.18+. This commit upgrades golangci-lint to the latest released version and fixes all linting errors that came up.
|
/retest |
1 similar comment
|
/retest |
chrischdi
left a comment
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
|
/lgtm |
|
pass! hurray! |
|
/cherry-pick release-0.5 third time's a charm |
|
/cherry-pick release-0.5 Third time also didn't work and I forgot about this afterwards, we are currently hitting this issue on #1204. |
|
@apricote: #1184 failed to apply on top of branch "release-0.5": In response to this:
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. |
Using strings.Title is deprecated in Go 1.18. This change is similar to kubernetes-sigs/cluster-api-provider-openstack#1184. Signed-off-by: Johanan Liebermann <[email protected]>
Using strings.Title is deprecated in Go 1.18. This change is similar to kubernetes-sigs/cluster-api-provider-openstack#1184. Signed-off-by: Johanan Liebermann <[email protected]>
What this PR does / why we need it:
With versions lower than 1.45.0, golangci-lint returns a lot of invalid errors when running in Go 1.18+. Because of this, all PR pipeline jobs
pull-cluster-api-provider-openstack-testfail.This commit upgrades golangci-lint to the latest released version and fixes all linting errors that came up.
Special notes for your reviewer:
/hold