[Umbrella Issue] GolangCI lint

[Adirio]

GolangCI uses several different linters concurrently to asses the code quality of this repository.

This issue lists which ones are implemented in this project:

• bodyclose
• deadcode: Add lint check and fix min required issues #1205
• depguard
• dogsled
• dupl: Add lint check and fix min required issues #1205
• errcheck: Add lint check and fix min required issues #1205
• funlen
• gochecknoglobals
• gochecknoinits
• gocognit
• goconst: 🏃 enable lint goconst #1294
• gocritic
• gocyclo: 🏃 enable gocycle lint check #1302
• godox
• gofmt: Delegate go vet and go fmt to golangci-lint #1310
• goimports: 🏃 Enable the lint check goimports  #1292
• golint: enable lint golint and fix notifications #1291
• gomnd
• goprintffuncname
• gosec: enable gosec lint check #1304
• gosimple: add missing lints to keep enabled all default ones #1312
• govet: Delegate go vet and go fmt to golangci-lint #1310
• ineffassign: Add lint check and fix min required issues #1205
• interfacer: Add lint check and fix min required issues #1205
• lll: enable golint lll check #1305
• maligned: enable maligned lint check #1293
• misspell: Add lint check and fix min required issues #1205
• nakedret: Add lint check and fix min required issues #1205
• prealloc: Enable linters for golangci-lint #1335
• rowserrcheck
• scopelint: Enable linters for golangci-lint #1335
• staticcheck: add missing lints to keep enabled all default ones #1312
• structcheck: Add lint check and fix min required issues #1205
• stylecheck
• typecheck: Enable linters for golangci-lint #1335
• unconvert; Enable linters for golangci-lint #1335
• unparam: Add lint check and fix min required issues #1205
• unused: add missing lints to keep enabled all default ones #1312
• varcheck: Add lint check and fix min required issues #1205
• whitespace
• wsl

Linters descriptions for reference

• govet - Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
• errcheck - Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases
• staticcheck - Staticcheck is a go vet on steroids, applying a ton of static analysis checks
• unused - Checks Go code for unused constants, variables, functions and types
• gosimple - Linter for Go source code that specializes in simplifying a code
• structcheck - Finds unused struct fields
• varcheck - Finds unused global variables and constants
• ineffassign - Detects when assignments to existing variables are not used
• deadcode - Finds unused code
• typecheck - Like the front-end of a Go compiler, parses and type-checks Go code
• bodyclose - checks whether HTTP response body is closed successfully
• golint - Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes
• rowserrcheck - checks whether Err of rows is checked successfully
• stylecheck - Stylecheck is a replacement for golint
• gosec - Inspects source code for security problems
• interfacer - Linter that suggests narrower interface types
• unconvert - Remove unnecessary type conversions
• dupl - Tool for code clone detection
• goconst - Finds repeated strings that could be replaced by a constant
• gocyclo - Computes and checks the cyclomatic complexity of functions
• gocognit - Computes and checks the cognitive complexity of functions
• gofmt - Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification
• goimports - Goimports does everything that gofmt does. Additionally it checks unused imports
• maligned - Tool to detect Go structs that would take less memory if their fields were sorted
• depguard - Go linter that checks if package imports are in a list of acceptable packages
• misspell - Finds commonly misspelled English words in comments
• lll - Reports long lines
• unparam - Reports unused function parameters
• dogsled - Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f())
• nakedret - Finds naked returns in functions greater than a specified function length
• prealloc - Finds slice declarations that could potentially be preallocated
• scopelint - Scopelint checks for unpinned variables in go programs
• gocritic - The most opinionated Go source code linter
• gochecknoinits - Checks that no init functions are present in Go code
• gochecknoglobals - Checks that no globals are present in Go code
• godox - Tool for detection of FIXME, TODO and other comment keywords
• funlen - Tool for detection of long functions
• whitespace - Tool for detection of leading and trailing whitespace
• wsl - Whitespace Linter - Forces you to use empty lines!
• goprintffuncname - Checks that printf-like functions are named with f at the end
• gomnd - An analyzer to detect magic numbers.

[camilamacedo86]

HI @Adirio,

I am not sure if we need to use all checks. regards the missing ones, do you think that all would be really valid to be added or could we close this one?

[Adirio]

I don't think we need them all, but this list will keep them tracked to know which ones we have and which we don't.
I think there is no issue with keeping this Issue opened for that reason.

[camilamacedo86]

Sorry, I am not sure if I understood. So, what is missing to address here for we are able to close this issue?

[Adirio]

The question is, do we really need to close this? If we do we can probably create a tag or any other way to group this kind of issues that track changes so that we can search them easily when needed even if they are closed.

[camilamacedo86]

The tasks represent the needs/requirements that need to be addressed. So, if it has nothing more that should be done because is obsoleted or referred, for example, then the next step would be close it and describe why. See that any task that has no activity for a while will also be closed by the boot.

So, why you would like to keep this open? What is missing to be done here?

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[Adirio]

/lifecycle frozen

[camilamacedo86]

HI @Adirio,

the open issues mean needs/tasks that we need to work on. IMO we have in place all lint checks that we need to. So, could you please clarify what in you POV is missing to get done for we close this one?

[Adirio]

Technically, we haven't added all linting checks, so we can't say it is really closed. Is it really a problem to leave it open and frozen?

[camilamacedo86]

HI @Adirio,

Has no problem to leave open any task that has actions that should get done but we should close all tasks which are done, or get outdated, were reject and etc. In this case, would we really like to add all possible lint checks? I do not think so. And then, over the lints checks which follow that were not added in the project has anyone here that you think that should be added?

IMO I do see a reason to add none of the following lint checks and then, in this way, this task should be closed. So, if you would like to keep it open what should be addressed here? What lint still required to be added and why?

• bodyclose
• depguard
• dogsled
• funlen
• gochecknoglobals
• gochecknoinits
• gocognit
• gocritic
• godox
• gomnd
• goprintffuncname
• rowserrcheck
• stylecheck
• whitespace
• wsl

[Adirio]

I don't see the need to add them right now but we may want want to add some more in the future. This issue just kept track of which were added and in which PR. But if you want we can close this issue.