Skip to content
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

Fix linter deprecation warnings and enhance coverage #241

Merged
merged 5 commits into from
Jun 1, 2024
Merged

Conversation

thomas11
Copy link
Contributor

Fix some linter deprecation warnings and add the gosimple linter. Can be reviewed commit by commit.

  • Silence golangci-lint warnings

WARN [config_reader] The configuration option run.skip-dirs is deprecated, please use issues.exclude-dirs.
WARN [config_reader] The configuration option run.skip-dirs-use-default is deprecated, please use issues.exclude-dirs-use-default.
WARN [lintersdb] The linter named "megacheck" is deprecated. It has been split into: gosimple, staticcheck, unused.

  • Add gosimple linter. Enabled linters now add up to the previous megacheck.
  • Fix gosimple warnings in the code base

@thomas11 thomas11 requested review from iwahbe and a team May 31, 2024 13:21
Copy link
Contributor

@blampe blampe left a comment

Choose a reason for hiding this comment

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

How would people feel about ci-mgmt plopping down some standardized lint rules for all the provider repos? It would mean some cleanup but it would be nice to have some standard conventions.

@@ -16,15 +17,15 @@ linters:
- gofmt
- revive
- gosec
- gosimple
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

- govet
- ineffassign
- lll
- misspell
- nakedret
- unconvert
- paralleltest
- staticcheck
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines +326 to +328
type contextKey string

var migrationsKey = contextKey("migrations")
Copy link
Contributor

Choose a reason for hiding this comment

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

Slightly easier to just use an empty struct since you don't need to do anything with it. Zero bytes as a bonus.

var migrationsKey struct{}

Copy link
Member

@iwahbe iwahbe left a comment

Choose a reason for hiding this comment

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

Thank you!

@thomas11
Copy link
Contributor Author

thomas11 commented Jun 1, 2024

How would people feel about ci-mgmt plopping down some standardized lint rules for all the provider repos? It would mean some cleanup but it would be nice to have some standard conventions.

Sure. Except this repo isn't under ci-mgmt at all currently...

@thomas11 thomas11 merged commit 2bf8636 into main Jun 1, 2024
6 checks passed
@thomas11 thomas11 deleted the tkappler/lint branch June 1, 2024 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants