kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle
https://cluster-api.sigs.k8s.io
Apache License 2.0
3.5k stars 1.3k forks source link

golangci-lint configuration should not use enable-all #4243

Closed CecileRobertMichon closed 3 years ago

CecileRobertMichon commented 3 years ago

https://github.com/kubernetes-sigs/cluster-api/blob/master/.golangci.yml currently uses enable-all: true / disable: [...]

We should switch it to use disable-all: true / enable: [...].

The golangci-lint documentation states:

please, do not use enable-all: it's deprecated and will be removed soon. inverted configuration with enable-all and disable is not scalable during updates of golangci-lint

https://golangci-lint.run/usage/configuration/

CecileRobertMichon commented 3 years ago

/help /good-first-issue

k8s-ci-robot commented 3 years ago

@CecileRobertMichon: This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to [this](https://github.com/kubernetes-sigs/cluster-api/issues/4243): >/help >/good-first-issue Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
ShrillShrestha commented 3 years ago

Hey @CecileRobertMichon, I am new to open source and trying to learn to contribute. I would love to work on the issue. Just a quick question, do we just need to change the .golangci.yml file? And under that, do I just need to change enable-all -> disable-all and enable -> disable? Or do I need to look over additional stuff?

CecileRobertMichon commented 3 years ago

@ShrillShrestha you should only have to change the .golangci.yml. The goal is to change the configuration syntax but to keep the exact same linters enabled. You can see which ones are currently enabled by running make lint:

INFO [lintersdb] Active 33 linters: [asciicheck bodyclose deadcode depguard dogsled errcheck goconst gocritic gocyclo gofmt goimports golint gomodguard goprintffuncname gosec gosimple govet ineffassign maligned misspell nakedret nolintlint prealloc rowserrcheck scopelint staticcheck structcheck stylecheck testpackage typecheck unconvert unparam varcheck]

I recommend running it before and after changing the configuration to using "enabled" to make sure the linters haven't changed.

ShrillShrestha commented 3 years ago

Thank @CecileRobertMichon, I will try to make that changes. I will let you know if I have further questions.