From 248100ce6dfa91a2878cc397d0ed9a18058f130c Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 1 Nov 2023 14:40:07 +0100 Subject: [PATCH] golangci-lint: tone down comment checking 39df946c06 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. --- hack/golangci-hints.yaml | 10 ++++++++++ hack/golangci-strict.yaml | 16 ++++++++++------ hack/golangci.yaml | 16 ++++++++++------ hack/golangci.yaml.in | 16 ++++++++++------ 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/hack/golangci-hints.yaml b/hack/golangci-hints.yaml index b94e5c510b0..82c976d4a94 100644 --- a/hack/golangci-hints.yaml +++ b/hack/golangci-hints.yaml @@ -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 + - linters: + - golint + - revive + - stylecheck + text: comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form|comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form|exported (.+) should have comment( \(or a comment on this block\))? or be unexported|package comment should be of the form "(.+)...|comment on exported (.+) should be of the form "(.+)...|should have a package comment + path-except: cmd/kubeadm + linters: disable-all: false enable: # please keep this alphabetized diff --git a/hack/golangci-strict.yaml b/hack/golangci-strict.yaml index 245765381a1..29014b3b474 100644 --- a/hack/golangci-strict.yaml +++ b/hack/golangci-strict.yaml @@ -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 + - linters: + - golint + - revive + - stylecheck + text: comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form|comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form|exported (.+) should have comment( \(or a comment on this block\))? or be unexported|package comment should be of the form "(.+)...|comment on exported (.+) should be of the form "(.+)...|should have a package comment + path-except: cmd/kubeadm + # The following issues were deemed "might be worth fixing, needs to be # decided on a case-by-case basis". This was initially decided by a # majority of the developers who voted in @@ -93,12 +103,6 @@ issues: - gosimple text: "S1033: unnecessary guard around call to delete" - # Only packages listed here opt into the strict "exported symbols must be documented". - - linters: - - revive - text: "(exported: exported .* or be unexported|exported: comment on exported.*should be of the form)" - path-except: cmd/kubeadm - # Didn't make it into https://github.com/kubernetes/kubernetes/issues/117288. # Discussion on Slack concluded that "it's hard to have a universal policy for all # functions marked deprecated" and thus this can only be a hint which must diff --git a/hack/golangci.yaml b/hack/golangci.yaml index 7ae6477ef20..4c511c912d2 100644 --- a/hack/golangci.yaml +++ b/hack/golangci.yaml @@ -73,6 +73,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 + - linters: + - golint + - revive + - stylecheck + text: comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form|comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form|exported (.+) should have comment( \(or a comment on this block\))? or be unexported|package comment should be of the form "(.+)...|comment on exported (.+) should be of the form "(.+)...|should have a package comment + path-except: cmd/kubeadm + # The following issues were deemed "might be worth fixing, needs to be # decided on a case-by-case basis". This was initially decided by a # majority of the developers who voted in @@ -99,12 +109,6 @@ issues: - gosimple text: "S1033: unnecessary guard around call to delete" - # Only packages listed here opt into the strict "exported symbols must be documented". - - linters: - - revive - text: "(exported: exported .* or be unexported|exported: comment on exported.*should be of the form)" - path-except: cmd/kubeadm - # Didn't make it into https://github.com/kubernetes/kubernetes/issues/117288. # Discussion on Slack concluded that "it's hard to have a universal policy for all # functions marked deprecated" and thus this can only be a hint which must diff --git a/hack/golangci.yaml.in b/hack/golangci.yaml.in index 4ccee2ad4e0..319bd621f7b 100644 --- a/hack/golangci.yaml.in +++ b/hack/golangci.yaml.in @@ -76,6 +76,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 + - linters: + - golint + - revive + - stylecheck + text: comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form|comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form|exported (.+) should have comment( \(or a comment on this block\))? or be unexported|package comment should be of the form "(.+)...|comment on exported (.+) should be of the form "(.+)...|should have a package comment + path-except: cmd/kubeadm + {{- if not .Hints}} # The following issues were deemed "might be worth fixing, needs to be @@ -104,12 +114,6 @@ issues: - gosimple text: "S1033: unnecessary guard around call to delete" - # Only packages listed here opt into the strict "exported symbols must be documented". - - linters: - - revive - text: "(exported: exported .* or be unexported|exported: comment on exported.*should be of the form)" - path-except: cmd/kubeadm - # Didn't make it into https://github.com/kubernetes/kubernetes/issues/117288. # Discussion on Slack concluded that "it's hard to have a universal policy for all # functions marked deprecated" and thus this can only be a hint which must