Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

lint: replace deprecated golang.org/x/lint/golint with golangci-lint #95

Closed
wants to merge 1 commit into from

Conversation

pohly
Copy link

@pohly pohly commented Feb 3, 2023

golint is deprecated. golangci-lint is a more than adequate replacement. Because it combines different linters, a single invocation in the Makefile is enough and then produces consistent output for all of them.

It also gets enabled as a GitHub action. This will annotate pull requests.

The godox linter gets enabled as replacement for the manual "grep -i fixme".

For the sake of convenience, "make lint" installs golangci-lint. In contrast to golint before, this is done without polluting the go.mod file with the extra dependencies.

Fixes: #92

@codecov
Copy link

codecov bot commented Feb 3, 2023

Codecov Report

Merging #95 (a7c49f9) into master (70e025e) will not change coverage.
The diff coverage is n/a.

❗ Current head a7c49f9 differs from pull request most recent head 2b5723b. Consider uploading reports for the commit 2b5723b to get more accurate results

@@           Coverage Diff           @@
##           master      #95   +/-   ##
=======================================
  Coverage   94.92%   94.92%           
=======================================
  Files           5        5           
  Lines         138      138           
=======================================
  Hits          131      131           
  Misses          4        4           
  Partials        3        3           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@CLAassistant
Copy link

CLAassistant commented Feb 3, 2023

CLA assistant check
All committers have signed the CLA.


.PHONY: lint
lint: $(GOLINT)
@rm -rf lint.log
@echo "Checking formatting..."
@gofmt -d -s $(GO_FILES) 2>&1 | tee lint.log
Copy link

Choose a reason for hiding this comment

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

should the gofmt / go vet / fixme checks remain?

Copy link
Author

Choose a reason for hiding this comment

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

"govet" is enabled by default in golangci-lint. I explicitly enabled "godox" as replacement for the grep command.

What I got wrong is "gofmt": I thought it was enabled by default., but it isn't. Will fix....

... and of course it helps to actually include the .golangci.yml in the commit 🥵

@pohly pohly force-pushed the golangci-lint branch 2 times, most recently from a491b6e to 0848523 Compare February 3, 2023 17:52
golint is deprecated. golangci-lint is a more than adequate
replacement. Because it combines different linters, a single invocation in the
Makefile is enough and then produces consistent output for all of them.

It also gets enabled as a GitHub action. This will annotate pull requests.

The godox linter gets enabled as replacement for the manual "grep -i fixme".

For the sake of convenience, "make lint" installs golangci-lint. In contrast to
golint before, this is done without polluting the go.mod file with the extra
dependencies.
@pohly
Copy link
Author

pohly commented Feb 6, 2023

@prashantv: do you want me to rebase or shall I close this PR? Entirely up to you, I'm fine with both.

@prashantv
Copy link
Collaborator

@pohly Thanks for the PR, since revive is a drop-in replacement that runs faster, I put that up in #97.

I think let's keep both open to see what direction others prefer -- if golangci-lint is preferred, then this PR can be rebased.

@abhinav
Copy link
Collaborator

abhinav commented Feb 13, 2023

Thanks, @pohly.
This can probably be closed now that #97 is merged.
(golangci-lint is cool, but it does a lot more than golint. If we ever switch to it, we'll want to drop staticcheck as well since golangci-lint bundles it in too.)

@pohly
Copy link
Author

pohly commented Feb 13, 2023

Please don't forget to do a release 😄

@pohly pohly closed this Feb 13, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please drop unmaintained golang.org/x/lint dependency from go.mod
5 participants