Skip to content

Comments

ci: add golangci-lint for linting#3269

Merged
sukunrt merged 1 commit intomasterfrom
push-szzpvwlsuuny
Apr 14, 2025
Merged

ci: add golangci-lint for linting#3269
sukunrt merged 1 commit intomasterfrom
push-szzpvwlsuuny

Conversation

@sukunrt
Copy link
Member

@sukunrt sukunrt commented Apr 7, 2025

Configures ci to use golang-ci for linting.

The workflow is copied from quic-go. The os specific checks don't add too much value, I guess. I am fine with keeping only linux.

I couldn't get the unparam linter to detect unused parameters in functions. I guess it is aggressive about not raising failse positives. Using revive for unparam check.

I fear errcheck may be too much some times. We can disable / configure that one when we come across a change that warrants it.

See: https://github.com/libp2p/go-libp2p/actions/runs/14307422981/job/40094362132?pr=3269
for a failure.

See: #1843 for past discussion on this.

Fixes: #3248

@sukunrt sukunrt force-pushed the push-szzpvwlsuuny branch 4 times, most recently from 5165014 to a9a5125 Compare April 7, 2025 11:07
@sukunrt sukunrt marked this pull request as ready for review April 7, 2025 11:29
Copy link
Contributor

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

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

@galargh any reason golangci-lint isn't part of unified ci?

Other repos could also benefit from it

@sukunrt
Copy link
Member Author

sukunrt commented Apr 8, 2025

There's this discussion where not everyone's convinced they need this. ipld/ipld#92

@sukunrt sukunrt requested a review from guillaumemichel April 8, 2025 13:43
@galargh
Copy link
Member

galargh commented Apr 13, 2025

@galargh any reason golangci-lint isn't part of unified ci?

The current plan is to add it as an optional additional step that could be easily enabled via Unified CI's config. Once it's out, I'll make sure to update the configuration here.

@sukunrt sukunrt merged commit 4b79972 into master Apr 14, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable (more?) lint rules in ci

4 participants