Repository navigation
build: replace DeepSource with a stricter, pinned golangci-lint - #341
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The local lint command does not include CI’s timeout argument despite the stated requirement that both invocations match.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Replaces DeepSource with expanded, pinned golangci-lint CI checks and resolves the resulting diagnostics.
Changes:
- Expands golangci-lint defect detection across all modules.
- Propagates generator validation failures through protoc.
- Applies lint-driven error handling and cleanup fixes.
| File | Description |
|---|---|
.deepsource.toml |
Removes DeepSource configuration. |
.github/workflows/golangci-lint.yml |
Pins and expands CI linting. |
.golangci.yml |
Enables stricter defect linters. |
Makefile |
Updates the local lint command. |
doc/dev-guide.md |
Documents lint targets. |
cmd/protoc-gen-gorums/main.go |
Returns generation errors to protoc. |
cmd/protoc-gen-gorums/gengorums/gorums.go |
Converts fatal validation failures to errors. |
cmd/protoc-gen-gorums/gengorums/gorums_dev.go |
Propagates development-generation errors. |
cmd/protoc-gen-gorums/gengorums/gorums_bundle.go |
Applies type and conversion simplifications. |
internal/testprotos/failing_test.go |
Verifies protoc reports validation reasons. |
internal/testutils/mock/mock.go |
Preserves wrapped errors. |
internal/testutils/servers/servers_test.go |
Removes unused receivers. |
internal/conn/inbound_manager.go |
Removes an unnecessary conversion. |
internal/stream/session.go |
Reorders session context parameters. |
internal/stream/inbound.go |
Updates session construction. |
internal/stream/outbound.go |
Updates session construction. |
internal/stream/testhelpers.go |
Removes an unused receiver. |
internal/stream/session_test.go |
Updates session test calls. |
internal/stream/channel_test.go |
Updates session test calls. |
internal/stream/teardown_deadlock_test.go |
Updates session test calls. |
stream_dedup_inflight_test.go |
Blanks an unused handler parameter. |
stream_dedup_dispatch_test.go |
Blanks unused handler parameters. |
options_test.go |
Documents an intentional nil-argument pattern. |
benchkit/aggregate_test.go |
Uses length-based result validation. |
benchkit/summary_test.go |
Uses length-based result validation. |
benchkit/stats_test.go |
Uses length-based result validation. |
benchkit/cmd/sweep/csvio.go |
Handles wrapped EOF errors. |
benchkit/cmd/sweep/deploy.go |
Handles wrapped EOF errors. |
benchkit/cmd/sweep/degraded.go |
Simplifies zero initialization. |
benchkit/cmd/sweep/offsets.go |
Safely validates regex matches. |
Files not reviewed (1)
- cmd/protoc-gen-gorums/gengorums/gorums_bundle.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
meling
force-pushed
the
feature/golangci-lint
branch
2 times, most recently
from
October 3, 2026 21:25
647058a to
a190160
Compare
meling
force-pushed
the
feature/golangci-lint
branch
2 times, most recently
from
October 4, 2026 09:44
644907c to
d48cead
Compare
meling
force-pushed
the
feature/golangci-lint
branch
from
October 4, 2026 14:42
d48cead to
7717aaa
Compare
The generator called log.Fatal for a file with several services, a reserved identifier, or invalid method options, which exits the plugin from library code. GenerateFile and GenerateDevFiles now return these errors, and the plugin hands them to protoc, which reports them. Method options are validated before any code is generated.
newSession took its context after the endpoint and stream, against the Go convention that a context comes first.
Test helpers and handlers named receivers and parameters they never use. Leaving them blank shows at a glance which inputs a test ignores.
csvio and deploy compared errors with io.EOF directly, which misses a wrapped EOF; they now use errors.Is. Percentile checks in tests and the offset line parser guarded indexing with a nil check, which does not catch an empty slice; they now check the length. The degraded-node scan no longer initializes a value that every path overwrites.
Registration errors were formatted with %v, so callers could not match the underlying protoregistry error.
strconv.ParseInteger[uint32] already returns a uint32.
The bundler asserted a function's type to *types.Signature to read its receiver; types.Func.Signature returns it without an assertion. It also converted the formatted source, already a []byte, to []byte.
CI ran golangci-lint at the latest release and reported only issues new in a pull request, so existing defects stayed hidden and local runs could disagree with CI. CI now pins v2.13.2 and lints all three modules with the same arguments as make lint, which now runs the golangci-lint binary. The configuration adds linters that report likely defects rather than style: unchecked type assertions, error comparisons that break on wrapping, gocritic's diagnostic checks, and similar. Tests may compare errors by identity and assert types unchecked.
golangci-lint in CI now covers the checks worth keeping, and DeepSource could not be run locally before pushing. Its loop-variable capture check no longer applies since Go 1.22 gave each iteration its own variable.
meling
force-pushed
the
feature/golangci-lint
branch
from
October 7, 2026 18:16
7717aaa to
c48c132
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
make lint, withoutonly-new-issues..golangci.ymladds linters for likely defects:forcetypeassert,errorlint, gocritic diagnostics,nilerr,nilnesserr,unconvert,wastedassign, and others.log.Fatal,newSessiontakes its context first, unused test names are blank..deepsource.toml. VET-V0010 no longer applies since Go 1.22, and SCC-SA4006 was a false positive from Go 1.26'snew(expr).Stack created with GitHub Stacks CLI • Give Feedback 💬