Skip to content

Commit

Permalink
Improve golang ci lint (#18)
Browse files Browse the repository at this point in the history
Signed-off-by: Aniruddha Basak <[email protected]>
  • Loading branch information
aniruddha2000 authored Oct 6, 2023
1 parent 168960c commit fc1c9e1
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 194 deletions.
229 changes: 83 additions & 146 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,20 @@ linters:
- bidichk
- bodyclose
- containedctx
# - depguard
- dogsled
- contextcheck
- durationcheck
- errcheck
- errchkjson
- errname
- errorlint
- exhaustive
- exportloopref
- forcetypeassert
- gci
- goconst
- gocritic
- godot
- gofmt
- gofumpt
- goimports
- goprintffuncname
- gosec
Expand All @@ -25,37 +28,29 @@ linters:
- importas
- ineffassign
- loggercheck
- makezero
- misspell
- nakedret
- nilerr
- noctx
- nolintlint
- nosprintfhostport
- prealloc
- predeclared
- reassign
- revive
- rowserrcheck
- staticcheck
- stylecheck
- tagliatelle
- thelper
- tparallel
- typecheck
- unconvert
- unparam
- unused
- whitespace
- wrapcheck
- contextcheck
- errname
- errorlint
- exhaustive
- forcetypeassert
- gofmt
- gofumpt
- makezero
- nosprintfhostport
- tagliatelle
- tparallel
- usestdlibvars
- unused
- wastedassign
- wrapcheck

linters-settings:
godot:
Expand Down Expand Up @@ -85,58 +80,27 @@ linters-settings:
# Controller Runtime
- pkg: sigs.k8s.io/controller-runtime
alias: ctrl
gofumpt:
extra-rules: true
nolintlint:
allow-unused: false
allow-leading-space: false
require-specific: true
staticcheck:
go: "1.20"
go: "1.21"
stylecheck:
go: "1.20"
go: "1.21"
checks: ["all", "-ST1006"]
dot-import-whitelist:
- "github.com/onsi/gomega"
- "github.com/onsi/ginkgo/v2"
gosec:
excludes:
- G307 # Deferring unsafe method "Close" on type "\*os.File"
- G108 # Profiling endpoint is automatically exposed on /debug/pprof
- G106 # Allowing for insecure ssh
gocritic:
enabled-tags:
- diagnostic
- experimental
- style
- performance
disabled-checks:
- appendAssign
- dupImport # https://github.com/go-critic/go-critic/issues/845
- evalOrder
- ifElseChain
- octalLiteral
- regexpSimplify
- sloppyReassign
- truncateCmp
- typeDefFirst
- unnamedResult
- unnecessaryDefer
- whyNoLint
- wrapperFunc
- rangeValCopy
- hugeParam
unused:
go: "1.20"
wrapcheck:
ignoreSigs:
- status.Error(
- .Errorf(
- errors.New(
- errors.Unwrap(
- .Wrap(
- .Wrapf(
- .WithMessage(
- .WithMessagef(
- .WithStack(
- .Complete(
- experimental
- opinionated
revive:
enable-all-rules: true
rules:
Expand All @@ -145,11 +109,12 @@ linters-settings:
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#add-constant
- name: add-constant
severity: warning
disabled: false
disabled: true
arguments:
- maxLitCount: "3"
allowStrs: '""'
allowInts: "0,1,2,3,42,100"

# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#argument-limit
- name: argument-limit
severity: warning
Expand Down Expand Up @@ -242,113 +207,85 @@ linters-settings:
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#comment-spacings
- name: comment-spacings
disabled: true
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#add-constant
- name: add-constant
disabled: true
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#use-any
- name: use-any
disabled: true
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#deep-exit
- name: deep-exit
disabled: true
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#struct-tag
- name: struct-tag
disabled: true
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#receiver-naming
- name: receiver-naming
disabled: true
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#receiver-naming
- name: nested-structs
disabled: true
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#struct-tag
- name: struct-tag
disabled: true
unused:
go: "1.21"
usestdlibvars:
# Suggest the use of http.MethodXX.
# Default: true
http-method: true
# Suggest the use of http.StatusXX.
# Default: true
http-status-code: true
# Suggest the use of time.Weekday.String().
# Default: true
time-weekday: true
# Suggest the use of time.Month.String().
# Default: false
time-month: true
# Suggest the use of time.Layout.
# Default: false
time-layout: true
# Suggest the use of crypto.Hash.String().
# Default: false
crypto-hash: true
# Suggest the use of rpc.DefaultXXPath.
# Default: false
default-rpc-path: true
# Suggest the use of os.DevNull.
# Default: false
os-dev-null: true
# Suggest the use of sql.LevelXX.String().
# Default: false
sql-isolation-level: true
# Suggest the use of tls.SignatureScheme.String().
# Default: false
tls-signature-scheme: true
# Suggest the use of constant.Kind.String().
# Default: false
constant-kind: true
# Suggest the use of syslog.Priority.
# Default: false
syslog-priority: true
wrapcheck:
ignoreSigs:
- status.Error(
- .Errorf(
- errors.New(
- errors.Unwrap(
- .Wrap(
- .Wrapf(
- .WithMessage(
- .WithMessagef(
- .WithStack(
- .Complete(
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
# 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-rules:
- linters:
- wrapcheck
path: _test\.go
- 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: "(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"
# 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|test)\/.*\/conversion.*\.go$
- linters:
- revive
text: "var-naming: don't use underscores in Go names;"
path: .*(api|types|test)\/.*\/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|test)\/.*\/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$
# We don't care about defer in for loops in test files.
- linters:
- gocritic
text: "deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop"
path: _test\.go
run:
timeout: 10m
skip-files:
- "zz_generated.*\\.go$"
- ".*conversion.*\\.go$"
- "test/e2e/*\\.go$"
skip-dirs:
- "third_party"
- "tilt_modules"
go: "1.21"
allow-parallel-runners: true
modules-download-mode: readonly
modules-download-mode: vendor
skip-dirs:
- vendor$
- test/vendor$
18 changes: 7 additions & 11 deletions config/default/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@ commonLabels:
cluster.x-k8s.io/provider: "infrastructure-cluster-stack-operator"

resources:
- ../crd
- ../rbac
- ../manager
#- ../webhook
- ../certmanager
- ../crd
- ../rbac
- ../manager
- ../certmanager

patchesStrategicMerge:
- manager_config_patch.yaml
#- manager_webhook_patch.yaml
#- webhookcainjection_patch.yaml
- manager_pull_policy.yaml

- manager_config_patch.yaml
- manager_pull_policy.yaml
# vars:
# - name: CERTIFICATE_NAMESPACE # namespace of the certificate CR
# objref:
Expand All @@ -44,4 +40,4 @@ patchesStrategicMerge:
# objref:
# kind: Service
# version: v1
# name: webhook-service
# name: webhook-service
7 changes: 4 additions & 3 deletions internal/clusterstackrelease/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@ func Summary(csr *csov1alpha1.ClusterStackRelease) (csov1alpha1.ClusterStackRele

// if provider-specific work is done, we are left with applying objects
// We don't expect the condition to be not set at all, hence no else case here
if conditions.IsTrue(csr, csov1alpha1.ProviderClusterStackReleaseReadyCondition) {
switch {
case conditions.IsTrue(csr, csov1alpha1.ProviderClusterStackReleaseReadyCondition):
summary.Phase = csov1alpha1.ClusterStackReleasePhaseApplyingObjects
} else if conditions.IsTrue(csr, csov1alpha1.ClusterStackReleaseAssetsReadyCondition) {
case conditions.IsTrue(csr, csov1alpha1.ClusterStackReleaseAssetsReadyCondition):
summary.Phase = csov1alpha1.ClusterStackReleasePhaseProviderSpecificWork
} else if conditions.IsFalse(csr, csov1alpha1.ClusterStackReleaseAssetsReadyCondition) {
case conditions.IsFalse(csr, csov1alpha1.ClusterStackReleaseAssetsReadyCondition):
summary.Phase = csov1alpha1.ClusterStackReleasePhaseDownloadingAssets
}

Expand Down
Loading

0 comments on commit fc1c9e1

Please sign in to comment.