golangci-lint: enable doc comment checking for cmd/kubeadm

Some code owners might want this for specific packages, like cmd/kubeadm. This cannot be enabled for everything because: - a lot of existing code doesn't pass (-> can't be in base config) - a lot of packages don't need it (-> shouldn't even be a hint)
kubernetes · Oct 25, 2023 · 39df946 · 39df946
1 parent f8a4e34
commit 39df946
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 4 deletions.
diff --git a/hack/golangci-hints.yaml b/hack/golangci-hints.yaml
@@ -24,6 +24,15 @@ output:
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
+
+  # The default excludes disable the "should have comment or be unexported" check from revive.
+  # We want that to be enabled, therefore we have to disable all default excludes and
+  # add those back one-by-one that we want. See https://github.com/golangci/golangci-lint/issues/456#issuecomment-617470264
+  exclude-use-default: false
+  exclude:
+    # staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore
+    - ineffective break statement. Did you mean to break out of the outer loop
+
   # Excluding configuration per-path, per-linter, per-text and per-source
   exclude-rules:
     # exclude ineffassign linter for generated files for conversion
@@ -45,7 +54,8 @@ issues:
     # https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1514201592
     - linters:
         - stylecheck
-      text: "ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)"
+        - revive
+      text: "(ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)|exported: exported function (Convert|SetDefaults)_.* should be of the form)"
 
     # This check currently has some false positives (https://github.com/nunnatsa/ginkgolinter/issues/91).
     - linters:
@@ -66,6 +76,7 @@ linters:
     - govet
     - ineffassign
     - logcheck
+    - revive
     - staticcheck
     - stylecheck
     - unused
@@ -86,6 +97,12 @@ linters-settings: # please keep this alphabetized
     - p: \.Extract
       pkg: ^k8s\.io/client-go/applyconfigurations/
       msg: should not be used because managedFields was removed
+  revive:
+    # Only these rules are enabled.
+    rules:
+      - name: exported
+        arguments:
+        - disableStutteringCheck
   staticcheck:
     checks:
       - "all"
diff --git a/hack/golangci-strict.yaml b/hack/golangci-strict.yaml
@@ -24,6 +24,15 @@ output:
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
+
+  # The default excludes disable the "should have comment or be unexported" check from revive.
+  # We want that to be enabled, therefore we have to disable all default excludes and
+  # add those back one-by-one that we want. See https://github.com/golangci/golangci-lint/issues/456#issuecomment-617470264
+  exclude-use-default: false
+  exclude:
+    # staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore
+    - ineffective break statement. Did you mean to break out of the outer loop
+
   # Excluding configuration per-path, per-linter, per-text and per-source
   exclude-rules:
     # exclude ineffassign linter for generated files for conversion
@@ -45,7 +54,8 @@ issues:
     # https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1514201592
     - linters:
         - stylecheck
-      text: "ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)"
+        - revive
+      text: "(ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)|exported: exported function (Convert|SetDefaults)_.* should be of the form)"
 
     # This check currently has some false positives (https://github.com/nunnatsa/ginkgolinter/issues/91).
     - linters:
@@ -83,6 +93,12 @@ 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
@@ -110,6 +126,7 @@ linters:
     - govet
     - ineffassign
     - logcheck
+    - revive
     - staticcheck
     - stylecheck
     - unused
@@ -133,6 +150,12 @@ linters-settings: # please keep this alphabetized
     - p: \.Extract
       pkg: ^k8s\.io/client-go/applyconfigurations/
       msg: should not be used because managedFields was removed
+  revive:
+    # Only these rules are enabled.
+    rules:
+      - name: exported
+        arguments:
+        - disableStutteringCheck
   staticcheck:
     checks:
       - "all"
diff --git a/hack/golangci.yaml b/hack/golangci.yaml
@@ -24,6 +24,15 @@ output:
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
+
+  # The default excludes disable the "should have comment or be unexported" check from revive.
+  # We want that to be enabled, therefore we have to disable all default excludes and
+  # add those back one-by-one that we want. See https://github.com/golangci/golangci-lint/issues/456#issuecomment-617470264
+  exclude-use-default: false
+  exclude:
+    # staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore
+    - ineffective break statement. Did you mean to break out of the outer loop
+
   # Excluding configuration per-path, per-linter, per-text and per-source
   exclude-rules:
     # exclude ineffassign linter for generated files for conversion
@@ -51,7 +60,8 @@ issues:
     # https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1514201592
     - linters:
         - stylecheck
-      text: "ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)"
+        - revive
+      text: "(ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)|exported: exported function (Convert|SetDefaults)_.* should be of the form)"
 
     # This check currently has some false positives (https://github.com/nunnatsa/ginkgolinter/issues/91).
     - linters:
@@ -89,6 +99,12 @@ 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
@@ -116,6 +132,7 @@ linters:
     - govet
     - ineffassign
     - logcheck
+    - revive
     - staticcheck
     - stylecheck
     - unused
@@ -140,6 +157,12 @@ linters-settings: # please keep this alphabetized
     enabled-checks:
       - equalFold
       - boolExprSimplify
+  revive:
+    # Only these rules are enabled.
+    rules:
+      - name: exported
+        arguments:
+        - disableStutteringCheck
   staticcheck:
     checks:
       - "all"

diff --git a/hack/golangci.yaml.in b/hack/golangci.yaml.in
@@ -24,6 +24,15 @@ output:
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
+
+  # The default excludes disable the "should have comment or be unexported" check from revive.
+  # We want that to be enabled, therefore we have to disable all default excludes and
+  # add those back one-by-one that we want. See https://github.com/golangci/golangci-lint/issues/456#issuecomment-617470264
+  exclude-use-default: false
+  exclude:
+    # staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore
+    - ineffective break statement. Did you mean to break out of the outer loop
+
   # Excluding configuration per-path, per-linter, per-text and per-source
   exclude-rules:
     # exclude ineffassign linter for generated files for conversion
@@ -54,7 +63,8 @@ issues:
     # https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1514201592
     - linters:
         - stylecheck
-      text: "ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)"
+        - revive
+      text: "(ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)|exported: exported function (Convert|SetDefaults)_.* should be of the form)"
 
     # This check currently has some false positives (https://github.com/nunnatsa/ginkgolinter/issues/91).
     - linters:
@@ -94,6 +104,12 @@ 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
@@ -122,6 +138,7 @@ linters:
     - govet
     - ineffassign
     - logcheck
+    - revive
     - staticcheck
     - stylecheck
     - unused
@@ -153,6 +170,12 @@ linters-settings: # please keep this alphabetized
       - equalFold
       - boolExprSimplify
   {{- end}}
+  revive:
+    # Only these rules are enabled.
+    rules:
+      - name: exported
+        arguments:
+        - disableStutteringCheck
   staticcheck:
     checks:
       - "all"