golangci-lint: tone down comment checking #121672
Conversation
39df946 was meant to enable stricter comment checking only for cmd/kubeadm because the maintainers of that want that. However, the exclude rule didn't capture all possible error texts and therefore some checks were enabled across the entire code base. The extended pattern is now based on the golangci-lint source code. Also, the hint config didn't suppress any of these checks.
k8s-ci-robot
added
the
release-note-none
k8s-ci-robot
added
kind/bug
|
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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. |
k8s-ci-robot
added
the
needs-priority
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pohly 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 |
k8s-ci-robot
added
the
approved
|
/retest |
|
/sig architecture @kubernetes/release-managers please note that this is a change in verifying our code base and not really a logic change going into end user. We had a bug in there which we are cleaning up (see PR body text) /milestone v1.29 |
k8s-ci-robot
added
sig/architecture
| @@ -67,6 +67,16 @@ issues: | |||
| - gocritic | |||
| text: "ifElseChain: rewrite if-else to switch statement" | |||
|
|
|||
| # Only packages listed here opt into the strict "exported symbols must be documented". | |||
| # | |||
| # Exclude texts from https://github.com/golangci/golangci-lint/blob/ab3c3cd69e602ff53bb4c3e2c188f0caeb80305d/pkg/config/issues.go#L11-L103 | |||
There was a problem hiding this comment.
/lgtm
/hold
drop hold, when see fit.
SGTM, but what would happen if the golangci defaults change, do we need to adapt?
There was a problem hiding this comment.
We might have to, although it should be rare.
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 52dfacce0d8887dde563f6d30ae6c780c30f031d
|
k8s-ci-robot
removed
the
do-not-merge/hold
What type of PR is this?
/kind bug
/kind failing-test
What this PR does / why we need it:
39df946 was meant to enable stricter comment checking only for cmd/kubeadm because the maintainers of that want that. However, the exclude rule didn't capture all possible error texts and therefore some checks were enabled across the entire code base.
The extended pattern is now based on the golangci-lint source code.
Also, the hint config didn't suppress any of these checks.
Which issue(s) this PR fixes:
Fixes #121660
Special notes for your reviewer:
I verified that #116516 and #121456 pass without issues, while errors in cmd/kubeadm are still caught.
Does this PR introduce a user-facing change?
/cc @aojea @neolit123 @kiashok