🌱 Bump golangci-lint to v1.47.1

[sbueringer]

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #6350

[sbueringer]

cc @killianmuldoon Finally :)

[killianmuldoon]

😆 you beat me to it - was just running it locally to make sure the gci fix is working.

[sbueringer]

laughing you beat me to it - was just running it locally to make sure the gci fix is working.

I didn't find a better way around the gci findings in condition_types.go. If you have a good idea.. :)

[stmcginnis]

/lgtm

[chrischdi]

/lgtm

🎉

[killianmuldoon]

/lgtm

I can't figure out why gci is dropping those comments, but I'm happy with the version for now until we get to the root cause. This change has been a long time coming 🙂

[sbueringer]

I dropped the 1.24 issue from "Fixes". There's another PR to bump to go 1.18

[sbueringer]

Looks like we're running into: golangci/golangci-lint-action#519

[oscr]

@sbueringer @killianmuldoon This issue is solved in gci v0.4.3. I just tried it. So I think this is a good solution until the next golangci-lint version is released daixiang0/gci#82

[sbueringer]

@sbueringer @killianmuldoon This issue is solved in gci v0.4.3. I just tried it. So I think this is a good solution until the next golangci-lint version is released daixiang0/gci#82

Thank you very much. I added a corresponding TODO.

[oscr]

gci v0.4.3 is now merged into golangci-lint. So next release of golangci-lint should fix this.

golangci/golangci-lint#2992

[killianmuldoon]

1.47.1 is out - it bumps the gci version and should have the fix: https://github.com/golangci/golangci-lint/releases/tag/v1.47.1

[oscr]

There is a new release of golangci-lint now with gci 0.4.3

https://github.com/golangci/golangci-lint/releases/tag/v1.47.1

[chrischdi]

/retitle 🌱 Bump golangci-lint to v1.47.1

[killianmuldoon]

/lgtm

[killianmuldoon]

/hold

We need to investigate why the linter is taking much longer to run with this version.

[sbueringer]

I'm on PTO starting tomorrow. Thx @killianmuldoon for taking over this PR!

[k8s-ci-robot]

New changes are detected. LGTM label has been removed.

[killianmuldoon]

Issue for the long runtime is open at the golangci-lint repo golangci/golangci-lint#2997

I'm trying to pinpoint which linter is responsible.

[Prajyot-Parab]

Possible way to fix - golangci/golangci-lint#2997 (comment)

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from sbueringer by writing /assign @sbueringer in a comment. 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:

• OWNERS

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

[killianmuldoon]

Thanks @Prajyot-Parab I tried this locally, it seemed to work once (but I was running golangci-lint through the debugger) but I couldn't reproduce with the flow being used in CAPI. I've bumped the version here to see if it works in CI while I try to debug.

[killianmuldoon]

The issue appears to be with revive - mgechev/revive#713 there's an issue up on the repo there so we'll wait and see how long it takes for a fix and a release of golangci-lint that includes it.

IMO it doesn't make sense to remove revive as it's a pretty big linter so re-enabling it in six months would likely result in a large diff. One compromise would be to update golangci-lint, disable revive and then add revive with a working, appropriate version as a separate job in our lint flow. That's not an elegant solution 😆 so I think we can afford to take a wait and see approach, figure out how long the release might take and decide in a couple of weeks whether we need to take action.

[ldez]

@killianmuldoon there is another workaround: disable the following revive rules: context-keys-type, errorf, modifies-value-receiver, range-val-address, string-of-int, time-equal, time-naming, unexported-return, unhandled-error, var-declaration.

[schrej]

Since the slow performance of revive is tracked as an issue, maybe add a comment to re-enable them once a future release improves performance?

[oscr]

New release out!

https://github.com/golangci/golangci-lint/releases/tag/v1.47.2 with change log: revive: ignore slow rules (https://github.com/golangci/golangci-lint/pull/2999)

[killianmuldoon]

My preference is to keep things really explicit and roll back the exceptions sooner rather than later. My preference is either:

1. Don't update for now and see if the bug can be fixed in the next couple of weeks
2. Update but keep the exclusions in the config so we can roll them back once a fix is out

@fabriziopandini WDYT?

[killianmuldoon]

/close
Let's deduplicate this work and continue to track it in #6737 as we try to find a solution to the linting issue.

[k8s-ci-robot]

@killianmuldoon: Closed this PR.

In response to this:

/close
Let's deduplicate this work and continue to track it in #6737 as we try to find a solution to the linting issue.

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.