-
Notifications
You must be signed in to change notification settings - Fork 146
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
lint: Switch to golangci-lint #210
Conversation
Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this @zeeke !
@adrianchiris , @eoghanlawless I added a minimal configuration, containing the linter list. Do you think it worths adding a full configuration like sriov-network-operator/.golangci.yml ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 the minimal configuration, if we need to add anything else to it in the future, it's a trivial change.
Makefile
Outdated
$(GOBIN)/golint: | $(BASE) ; $(info Installing golint...) | ||
$Q go install golang.org/x/lint/golint | ||
GOLANGCILINT = $(GOBIN)/golangci-lint | ||
$(GOBIN)/golangci-lint: | $(BASE) ; $(info Installing golangci-lint...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$(GOBIN)/golangci-lint: | $(BASE) ; $(info Installing golangci-lint...) | |
$(GOLANGCILINT): | $(BASE) ; $(info Installing golangci-lint...) |
@@ -0,0 +1,6 @@ | |||
linters: | |||
enable: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enable: | |
disable-all: true | |
enable: |
Do we need this here, it's in the network operator's .golangci.yml
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what is the best for this. Default enabled linters are the following:
golangci-lint help linters
Enabled by default linters:
deadcode: Finds unused code [fast: false, auto-fix: false]
errcheck: Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: false, auto-fix: false]
gosimple (megacheck): Linter for Go source code that specializes in simplifying a code [fast: false, auto-fix: false]
govet (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: false, auto-fix: false]
ineffassign: Detects when assignments to existing variables are not used [fast: true, auto-fix: false]
staticcheck (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: false, auto-fix: false]
structcheck: Finds unused struct fields [fast: false, auto-fix: false]
typecheck: Like the front-end of a Go compiler, parses and type-checks Go code [fast: false, auto-fix: false]
unused (megacheck): Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false]
varcheck: Finds unused global variables and constants [fast: false, auto-fix: false]
...
As long as we use a specific version of golangci-lint, the default linter list should not change. And I think it's good to get this list updated when updating the tool.
golint tool has been deprecated and its repository is archived. Use golangci-lint in makefile, as it's what is currently used in CI lanes. Switch golint sublinter to revive, as suggested by golangci-lint itself. Move the linter list to `.golangci.yml` file to avoid misalignments between CI configuration and Makefile `lint` rule. Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
This PR comes from this thread on #205
golint
has been deprecated and this PR replaces it withgolangci-lint
, in order to have the same lint output as CI.Also, it replace
golint
sublinter withrevive
, as suggested by golangci-lint in its output