Add revive to golanci #37
Conversation
bshewale
force-pushed
the
enable-new-linters
branch
2 times, most recently
from
68d70dc to
a8e843b
Compare
|
Hi. I'm the writer of ginkgolinter and I see here it's not working for you, and I would like to help you with it, if you want. I wonder how the pre-commit is working. Is it running the configuration from the main branch, or from the PR? Because ginkgolinter is a bit new in golangci-lint, maybe, if the pre-commit uses the main branch configuration, it's using an old version of golangci-lint, that does not contain the new linter, yet? even the old version will read the .golangci.yml file and will find the ginkgolinter there. The version in this PR (v1.52.2) should contain it. |
This PR replace deprecated go-linter using revive (https://golangci-lint.run/usage/linters/#revive). We will add "ginkgolinter" in follow up patch once we merged add pre-commit job in openshift/releases. If we add "ginkgolinter" in this patch then it's causes below issue [1] ``` level=info msg="[config_reader] Config search paths: [./ /home/prow/go/src/github.com/openstack-k8s-operators/dataplane-operator /home/prow/go/src/github.com/openstack-k8s-operators /home/prow/go/src/github.com /home/prow/go/src /home/prow/go /home/prow /home /]" level=info msg="[config_reader] Used config file .golangci.yaml" level=error msg="Running error: unknown linters: 'ginkgolinter', run 'golangci-lint help linters' to see the list of supported linters" ``` This patch also Remove fmt dependency from make tidy as per [2]: The fmt dependency cannot be run if the go.mod file needs an update: ``` $ make tidy go fmt ./... go: updates to go.mod needed; to update it: go mod tidy make: *** [Makefile:103: fmt] Error 1 ``` I have also made some changes as per the comment given here [3] [1]: https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openstack-k8s-operators_dataplane-operator/147/pull-ci-openstack-k8s-operators-dataplane-operator-main-golangci/1643555465537261568/artifacts/test/build-log.txt [2]: openstack-k8s-operators/placement-operator#153 [3]: openstack-k8s-operators/cinder-operator#147 (comment)
bshewale
force-pushed
the
enable-new-linters
branch
from
a8e843b to
b96944d
Compare
|
/test precommit-check |
|
@stuggi this is one ready, ptal when you have some time, thanks. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abays, bshewale The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| endpoint.EndpointAdmin: { | ||
| Port: designate.DesignateAdminPort, | ||
| }, |
There was a problem hiding this comment.
for all other services we stopped creating the admin endpt, e.g. https://github.com/openstack-k8s-operators/placement-operator/blob/master/controllers/placementapi_controller.go#L328-L334 . @weinimo ok to not have the admin endpt for designate?
There was a problem hiding this comment.
@dkehn what do you think?
(Sorry, I am not very familiar with Designate's design)
This PR replace deprecated go-linter using
revive (https://golangci-lint.run/usage/linters/#revive).
We will add "ginkgolinter" in follow up patch once we merged add pre-commit job in openshift/releases.
If we add "ginkgolinter" in this patch then it's causes below issue 1
This patch also Remove fmt dependency from make tidy as per 2:
The fmt dependency cannot be run if the go.mod file needs an update:
I have also made some changes as per the comment given here 3