Skip to content

Move golangci-lint to `make lint` and remove `make lint-all`

Marcin S. requested to merge fix-make-lint into master

MERGE REQUEST

Overview

Note: Before merging, we'll need to let people know in Discord that the lint-all make target is being removed, as people are using it in their git hooks.

Previously, make lint was catching errors that make lint-all was not. This was unexpected since "lint-all" should be linting, well, all. I fixed this by turning off default exclusions in the golangci-lint configuration, and I re-enabled some exclusions for particularly noisy lints.

Next, I noticed that make lint and make lint-all had a lot of overlap, so I consolidated them into one make target. The new target is very minimally slower -- make lint used to run go lint, now it runs golangci which runs other linters in parallel with go lint and is very performant. It uses the same confidence settings for go lint as previously.

Finally, golangci caught some errors using golint which were not being caught earlier. I don't know why go lint missed them earlier, but I fixed the errors.

Testing

To test, replicate some of these scenarios which golangci-lint should be catching via golint:

https://codeengineered.com/blog/2014/golint/

The specific case that golint was catching but golangci-lint was not was when the name of an exported item did not match its name in the comment.

Example for Visual Changes

Checklist

Review and complete the checklist to ensure that the MR is complete before assigned to an approver.

  • All new methods or updated methods have clear docstrings
  • Testing added or updated for new methods
  • Any new packages are added to Makefile and .gitlab-ci.yml
  • API documentation updated for API updates
  • Module README.md updated for changes to workflow
  • Issue added to Sia-UI repo for new supporting features
  • Changelog File Created

Issues Closed

Edited by Marcin S.

Merge request reports