-
Notifications
You must be signed in to change notification settings - Fork 25
chore(lint): bump golangci lint config to v2, address lint findings #365
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
1596e01
d9d9379
608024c
cc71d16
52372c3
4284480
852b4d5
ae2deb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,3 +52,7 @@ node_modules/ | |
|
|
||
| .cursor | ||
| .envrc | ||
|
|
||
| docker-compose.yaml | ||
| mise.toml | ||
| .jj/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| # Copyright 2024 StreamNative | ||
| # Copyright 2025 StreamNative | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
|
|
@@ -12,26 +12,26 @@ | |
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| version: "2" | ||
|
|
||
| run: | ||
| go: "1.24" | ||
| build-tags: | ||
| - tools | ||
| - e2e | ||
| allow-parallel-runners: true | ||
|
|
||
| linters: | ||
| disable-all: true | ||
| default: none | ||
| enable: | ||
| - asciicheck | ||
| - bodyclose | ||
| - unused | ||
| # - deadcode | ||
| # - depguard | ||
| - copyloopvar | ||
| - dogsled | ||
| - errcheck | ||
| - copyloopvar | ||
| # - gci | ||
| - gocritic | ||
| # - gocyclo | ||
| # - godot | ||
| - gofmt | ||
| - goimports | ||
| - goprintffuncname | ||
| - gosec | ||
| - gosimple | ||
| - govet | ||
| - importas | ||
| - ineffassign | ||
|
|
@@ -43,166 +43,148 @@ linters: | |
| - predeclared | ||
| - rowserrcheck | ||
| - staticcheck | ||
| # - structcheck | ||
| - stylecheck | ||
| - thelper | ||
| - typecheck | ||
| - unconvert | ||
| - unparam | ||
| # - varcheck | ||
|
|
||
| linters-settings: | ||
| godot: | ||
| # declarations - for top level declaration comments (default); | ||
| # toplevel - for top level comments; | ||
| # all - for all comments. | ||
| scope: toplevel | ||
| exclude: | ||
| - '^ \+.*' | ||
| - "^ ANCHOR.*" | ||
| gci: | ||
| sections: | ||
| - prefix(github.com/streamnative) | ||
| - unused | ||
| settings: | ||
| gocritic: | ||
| disabled-checks: | ||
| - appendAssign | ||
| - dupImport | ||
| - evalOrder | ||
| - ifElseChain | ||
| - octalLiteral | ||
| - regexpSimplify | ||
| - sloppyReassign | ||
| - truncateCmp | ||
| - typeDefFirst | ||
| - unnamedResult | ||
| - unnecessaryDefer | ||
| - whyNoLint | ||
| - wrapperFunc | ||
| enabled-tags: | ||
| - experimental | ||
| godot: | ||
| scope: toplevel | ||
| exclude: | ||
| - ^ \+.* | ||
| - ^ ANCHOR.* | ||
| gosec: | ||
| excludes: | ||
| - G307 | ||
| - G108 | ||
| importas: | ||
| alias: | ||
| - pkg: k8s.io/api/core/v1 | ||
| alias: corev1 | ||
| - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1 | ||
| alias: apiextensionsv1 | ||
| - pkg: k8s.io/apimachinery/pkg/apis/meta/v1 | ||
| alias: metav1 | ||
| - pkg: k8s.io/apimachinery/pkg/api/errors | ||
| alias: apierrors | ||
| - pkg: k8s.io/apimachinery/pkg/util/errors | ||
| alias: kerrors | ||
| - pkg: sigs.k8s.io/controller-runtime | ||
| alias: ctrl | ||
| no-unaliased: true | ||
| nolintlint: | ||
| require-specific: true | ||
| allow-unused: false | ||
| exclusions: | ||
| generated: lax | ||
| rules: | ||
| - linters: | ||
| - revive | ||
| text: 'exported: exported method .*\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported' | ||
| - linters: | ||
| - errcheck | ||
| text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked | ||
| - linters: | ||
| - revive | ||
| text: exported (method|function|type|const) (.+) should have comment or be unexported | ||
| source: (func|type).*Fake.* | ||
| - linters: | ||
| - revive | ||
| path: fake_\.go | ||
| text: exported (method|function|type|const) (.+) should have comment or be unexported | ||
| - linters: | ||
| - revive | ||
| path: cmd/clusterctl/internal/test/providers.*.go | ||
| text: exported (method|function|type|const) (.+) should have comment or be unexported | ||
| - linters: | ||
| - revive | ||
| path: (framework|e2e)/.*.go | ||
| text: exported (method|function|type|const) (.+) should have comment or be unexported | ||
| - linters: | ||
| - unparam | ||
| text: always receives | ||
| - path: _test\.go | ||
| text: should not use dot imports | ||
| - path: (framework|e2e)/.*.go | ||
| text: should not use dot imports | ||
| - path: _test\.go | ||
| text: cyclomatic complexity | ||
| - linters: | ||
| - gocritic | ||
| text: 'appendAssign: append result not assigned to the same slice' | ||
| - linters: | ||
| - ifshort | ||
| path: controllers/mdutil/util.go | ||
| text: variable .* is only used in the if-statement | ||
| - linters: | ||
| - staticcheck | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| text: 'SA1019: in.(.+) is deprecated' | ||
| - linters: | ||
| - revive | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| text: exported (method|function|type|const) (.+) should have comment or be unexported | ||
| - linters: | ||
| - revive | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| text: 'var-naming: don''t use underscores in Go names;' | ||
| - linters: | ||
| - revive | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| text: 'receiver-naming: receiver name' | ||
|
Comment on lines
+94
to
+143
|
||
| - linters: | ||
| - staticcheck | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| text: 'ST1003: should not use underscores in Go names;' | ||
| - linters: | ||
| - staticcheck | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| text: 'ST1016: methods on the same type should have the same receiver name' | ||
| - linters: | ||
|
mattsre marked this conversation as resolved.
Outdated
|
||
| - ifshort | ||
| path: ^controllers/machine_controller\.go$ | ||
| text: variable 'isDeleteNodeAllowed' is only used in the if-statement.* | ||
|
mattsre marked this conversation as resolved.
Outdated
|
||
| paths: | ||
| - zz_generated.*\.go$ | ||
| - third_party | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ | ||
|
|
||
| importas: | ||
| no-unaliased: true | ||
| alias: | ||
| # Kubernetes | ||
| - pkg: k8s.io/api/core/v1 | ||
| alias: corev1 | ||
| - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1 | ||
| alias: apiextensionsv1 | ||
| - pkg: k8s.io/apimachinery/pkg/apis/meta/v1 | ||
| alias: metav1 | ||
| - pkg: k8s.io/apimachinery/pkg/api/errors | ||
| alias: apierrors | ||
| - pkg: k8s.io/apimachinery/pkg/util/errors | ||
| alias: kerrors | ||
| # Controller Runtime | ||
| - pkg: sigs.k8s.io/controller-runtime | ||
| alias: ctrl | ||
| nolintlint: | ||
| allow-unused: false | ||
| require-specific: true | ||
| gosec: | ||
| excludes: | ||
| - G307 # Deferring unsafe method "Close" on type "\*os.File" | ||
| - G108 # Profiling endpoint is automatically exposed on /debug/pprof | ||
| gocritic: | ||
| enabled-tags: | ||
| - experimental | ||
| disabled-checks: | ||
| - appendAssign | ||
| - dupImport # https://github.com/go-critic/go-critic/issues/845 | ||
| - evalOrder | ||
| - ifElseChain | ||
| - octalLiteral | ||
| - regexpSimplify | ||
| - sloppyReassign | ||
| - truncateCmp | ||
| - typeDefFirst | ||
| - unnamedResult | ||
| - unnecessaryDefer | ||
| - whyNoLint | ||
| - wrapperFunc | ||
| issues: | ||
| max-same-issues: 0 | ||
| max-issues-per-linter: 0 | ||
| # We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant | ||
| # changes in PRs and avoid nitpicking. | ||
| exclude-use-default: false | ||
| exclude-files: | ||
| - "zz_generated.*\\.go$" | ||
| exclude-dirs: | ||
| - third_party | ||
| exclude-rules: | ||
| - linters: | ||
| - revive | ||
| text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported" | ||
| - linters: | ||
| - errcheck | ||
| text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked | ||
| # Exclude some packages or code to require comments, for example test code, or fake clients. | ||
| - linters: | ||
| - revive | ||
| text: exported (method|function|type|const) (.+) should have comment or be unexported | ||
| source: (func|type).*Fake.* | ||
| - linters: | ||
| - revive | ||
| text: exported (method|function|type|const) (.+) should have comment or be unexported | ||
| path: fake_\.go | ||
| - linters: | ||
| - revive | ||
| text: exported (method|function|type|const) (.+) should have comment or be unexported | ||
| path: cmd/clusterctl/internal/test/providers.*.go | ||
| - linters: | ||
| - revive | ||
| text: exported (method|function|type|const) (.+) should have comment or be unexported | ||
| path: "(framework|e2e)/.*.go" | ||
| # Disable unparam "always receives" which might not be really | ||
| # useful when building libraries. | ||
| - linters: | ||
| - unparam | ||
| text: always receives | ||
| # Dot imports for gomega or ginkgo are allowed | ||
| # within test files. | ||
| - path: _test\.go | ||
| text: should not use dot imports | ||
| - path: (framework|e2e)/.*.go | ||
| text: should not use dot imports | ||
| - path: _test\.go | ||
| text: cyclomatic complexity | ||
| # Append should be able to assign to a different var/slice. | ||
| - linters: | ||
| - gocritic | ||
| text: "appendAssign: append result not assigned to the same slice" | ||
| # ifshort flags variables that are only used in the if-statement even though there is | ||
| # already a SimpleStmt being used in the if-statement in question. | ||
| - linters: | ||
| - ifshort | ||
| text: "variable .* is only used in the if-statement" | ||
| path: controllers/mdutil/util.go | ||
| # Disable linters for conversion | ||
| - linters: | ||
| - staticcheck | ||
| text: "SA1019: in.(.+) is deprecated" | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| - linters: | ||
| - revive | ||
| text: exported (method|function|type|const) (.+) should have comment or be unexported | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| - linters: | ||
| - revive | ||
| text: "var-naming: don't use underscores in Go names;" | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| - linters: | ||
| - revive | ||
| text: "receiver-naming: receiver name" | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| - linters: | ||
| - stylecheck | ||
| text: "ST1003: should not use underscores in Go names;" | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| - linters: | ||
| - stylecheck | ||
| text: "ST1016: methods on the same type should have the same receiver name" | ||
| path: .*(api|types)\/.*\/conversion.*\.go$ | ||
| # hack/tools | ||
| - linters: | ||
| - typecheck | ||
| text: import (".+") is a program, not an importable package | ||
| path: ^tools\.go$ | ||
| # Ignore ifshort false positive | ||
| # TODO(sbueringer) false positive: https://github.com/esimonov/ifshort/issues/23 | ||
| - linters: | ||
| - ifshort | ||
| text: "variable 'isDeleteNodeAllowed' is only used in the if-statement.*" | ||
| path: ^controllers/machine_controller\.go$ | ||
| max-same-issues: 0 | ||
|
|
||
| run: | ||
| timeout: 10m | ||
| build-tags: | ||
| - tools | ||
| - e2e | ||
| allow-parallel-runners: true | ||
| go: "1.24" | ||
| formatters: | ||
| enable: | ||
| - gofmt | ||
| - goimports | ||
| settings: | ||
| gci: | ||
| sections: | ||
| - prefix(github.com/streamnative) | ||
| exclusions: | ||
| generated: lax | ||
| paths: | ||
| - zz_generated.*\.go$ | ||
| - third_party | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ | ||
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.
The
godotlinter has settings configured but is not enabled in the linters list (line 26). These settings will have no effect. Ifgodotlinting is desired, add it to the enabled linters list. Otherwise, consider removing these unused settings to keep the configuration clean.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.
Removing
godotlinter config -godotis used for adding "." at the end of sentenences however most of the comments/docs in the repo do not follow this pattern so it doesn't seem like this should be enabled