Skip to content

dev: refactor .golangci.yml configuration and fix up nolintlint issues #4537

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

Merged
merged 8 commits into from
Mar 19, 2024

Conversation

alexandear
Copy link
Member

The PR fixes nolintlint issues and refactors nolintlint configuration for golangci-lint repo.

Main changes:

  • Ignore dupl and lll for tests and remove //nolint:dupl, //nolint:lll. Usually, it's OK to have duplicated code and long lines in tests.
  • Ignore dupl for the pkg/golinters package and remove //nolint:dupl. Because the logic of creating linters is similar between linters.
  • Ignore misspelled importas because it's a linter.
  • Fix easy-to-fix gocritic issues and remove //nolint:gocritic comments.
  • Add explanation comments for //nolint:<linter>.

One commit one change.

@alexandear alexandear added the topic: cleanup Related to code, process, or doc cleanup label Mar 19, 2024
@ldez ldez added this to the next milestone Mar 19, 2024
It's usually OK to have long lines and duplicated code in tests.
Because the creation logic is similar between linters.
pkg/golinters/goanalysis/runner.go:180:1: unnamedResult: consider giving a name to these results (gocritic)
func (r *runner) prepareAnalysis(pkgs []*packages.Package,
^
pkg/golinters/goanalysis/runners.go:223:5: rangeValCopy: each iteration copies 128 bytes (consider pointers or indexing) (gocritic)
                                for _, i := range pkgIssues {
                                ^
pkg/golinters/goanalysis/runners.go:187:1: unnamedResult: consider giving a name to these results (gocritic)
func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context,
^
pkg/golinters/gofmt_common.go:136:33: appendAssign: append result not assigned to the same slice (gocritic)
                change.Replacement.NewLines = append(p.replacementLinesToPrepend, addedLines...)
                                              ^
Change nolintlint configuration:
  - require an explanation for nolint directives
  - to be specific about which linter is being skipped
Fix up nolintlint issues by adding explanation comments.
@ldez ldez force-pushed the feat/refactor-nolintlint branch from 24c13aa to 6fd95fa Compare March 19, 2024 13:56
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ldez ldez merged commit 6709c97 into golangci:master Mar 19, 2024
@alexandear alexandear deleted the feat/refactor-nolintlint branch March 19, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: cleanup Related to code, process, or doc cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants