snapd
snapd copied to clipboard
run-checks: add `golangci-lint` to static checks
golangci-lint
is run as part of Github workflows on pull request, but it is not run as part of local testing via ./run-checks
. This PR proposes adding it to ./run-checks
, with automatic installation (assuming curl
or wget
).
Installation instructions for golangci-lint
can be found here: https://golangci-lint.run/usage/install/#local-installation
Some other possibilities:
- Check the sha256 hash of the installed binary to make sure it has not been tampered with
- Install
golangci-lint
to/tmp
or some other workspace which is later discarded (would make testing fail without internet -- bad)
This relates to the commit here (https://github.com/snapcore/snapd/pull/13040) which fixes lint errors introduced here (https://github.com/snapcore/snapd/commit/b27425ab99c47cf8fc1b3bb8b0df96d143ea6fa3) which were not caught by local tests before pushing to master.
Would it be preferable to use the snap of golangci-lint instead? I maintain this currently myself but would be happy to transfer it to Canonical and maintain it there instead if desired.
Codecov Report
Merging #13042 (b66395a) into master (661258f) will increase coverage by
0.00%
. The diff coverage isn/a
.
:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
@@ Coverage Diff @@
## master #13042 +/- ##
=======================================
Coverage 78.74% 78.74%
=======================================
Files 1002 1002
Lines 124950 124950
=======================================
+ Hits 98392 98395 +3
+ Misses 20377 20375 -2
+ Partials 6181 6180 -1
Flag | Coverage Δ | |
---|---|---|
unittests | 78.74% <ø> (+<0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
Would it be preferable to use the snap of golangci-lint instead? I maintain this currently myself but would be happy to transfer it to Canonical and maintain it there instead if desired.
Thank you for the offer! That would certainly be the easiest solution. I think the most up-to-date version of golangci-lint
shipped in the snap is more strict than the version currently used in the CI checks. I'm somewhat worried about ./run-checks
failing as a result of a golangci-lint snap update, when no code changes have occurred in snapd. This would be particularly problematic when working on changes for a PR when an update occurs and various orthogonal fixes are required to fix linting errors before the work which is the focus of the PR can be continued. Thus, my thought was to match the version used for the CI. But perhaps it wouldn't be a bad thing to be more strict than the CI.
Fwiw, I like the idea of using the snap better than using curl | bash
. We could use "snap install --revision=xxx" if we want to pin to specific versions (or use tracks) if this is a concern.