chore: replace make vet and fmt with golangci-lint #12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In CI, this project already uses golangci-lint to configure and run all of its various linters. Run the same thing locally in "make lint" and as part of make targets that previously ran "go vet" and "go fmt". This has the added benefit of commands like "make build" and "make test" causing diffs, since it is difficut to run "go fmt" without it actually modifying files. Add a new "golangci-lint" target that downloads a project-specific copy of golangci-lint to the project's bin directory, similar to what we do for kustomize and other tools. The version is set to v1.52.2, which is the same we have configured in CI. Add a new "make lint" target to run the linter and get rid of the old "vet" and "fmt" targets since our lint list checks those. Call this target from everywhere that previously called "vet" and "fmt".
Avoid the situation where our CI runs lint as part of its own action, and then again implicitly as part of running builds or unit tests. CI will no only run lint in its own action. Remove the "test" make target as a dependency of the "docker-build" target, and the "lint" make target as a dependency of the "build" target. Instead, CI now explicitly calls a new target "make test-nolint" when running unit tests. This target runs everything that "make test" used to run except for linting. "make test" still exists for command-line usage. Note that the "build" make target is not explicitly called in CI, but it no longer depends upon "lint" so that it acts like the other targets that perform builds.
scottd018
approved these changes
May 9, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In CI, this project already uses golangci-lint to configure and run all of its various linters. Run the same thing locally in "make lint" and as part of make targets that previously ran "go vet" and "go fmt". This has the added benefit of commands like "make build" and "make test" causing diffs, since it is difficut to run "go fmt" without it actually modifying files.
Add a new "golangci-lint" target that downloads a project-specific copy of golangci-lint to the project's bin directory, similar to what we do for kustomize and other tools. The version is set to v1.52.2, which is the same we have configured in CI.
Add a new "make lint" target to run the linter and get rid of the old "vet" and "fmt" targets since our lint list checks those. Also restructure the make targets and CI slightly so that when CI runs builds, it doesn't implicitly run the linter again, since it has already done so in a previous action.