Skip to content

Update golangci-lint to v2.15.0 to fix flaky revive results - #580

Merged
oschwald merged 5 commits into
mainfrom
wstorey/stf-2212-fix-flaky-golangci-lint-revive-results-in-geoipupdate
Oct 26, 2026
Merged

oschwald merged 5 commits into
mainfrom
wstorey/stf-2212-fix-flaky-golangci-lint-revive-results-in-geoipupdate

Conversation

@horgh

@horgh horgh commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

The lint job on main failed once with nolintlint errors for the //nolint:revive directives in cmd/geoipupdate/args.go (run). The next run passed with the same golangci-lint version (2.12.2).

Cause

golangci-lint v2.12.2 uses revive v1.15.0. In that version, the epoch-naming and inefficient-map-lookup rules cannot type check a package that imports non-stdlib packages. They return an internal failure, and revive then skips all remaining rules for that file. golangci-lint drops the internal failure without a message. Revive runs the rules in random order, so deep-exit sometimes does not run on args.go.

golangci-lint v2.15.0 uses revive v1.17.1, which logs the internal failure and continues with the other rules.

Changes

  • Update golangci-lint to v2.15.0. This commit also disables exhaustruct_v5, replaces the deprecated gofumpt extra-rules setting, and adds a gosec nolint directive for the user-specified *_FILE path. The upgrade cannot pass lint without these changes.
  • Fix new issues from v2.15.0 in separate commits:
    • Remove the redundant go1.12 build tag.
    • Use errors.AsType in IsRetryableError.
    • Use a value receiver for ReadResult.MarshalJSON. The --output JSON does not change.
  • Enable gofumpt balance-calls.

Each commit passes lint and tests with the golangci-lint version that its mise.lock pins.

Linear: STF-2212

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated code quality checks and formatting rules.
    • Made small maintenance updates to error handling and JSON serialization.
    • Adjusted build configuration and test formatting. No changes to retry decisions or test behavior.
  • User Impact
    • No new user-facing features are included in this update.

horgh and others added 5 commits October 26, 2026 21:24
The module requires Go 1.26, so the go1.12 build constraint in
version.go has no effect.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With a value receiver, encoding/json uses MarshalJSON for both
ReadResult values and pointers. For a nil pointer, encoding/json
writes null, so the nil check is not necessary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The lint job on main failed once with nolintlint errors for the
//nolint:revive directives in cmd/geoipupdate/args.go. The next run
passed with the same golangci-lint version (2.12.2).

golangci-lint v2.12.2 uses revive v1.15.0. In that version, the
epoch-naming and inefficient-map-lookup rules cannot type check a
package that imports non-stdlib packages. They return an internal
failure, and revive then skips all remaining rules for that file.
golangci-lint drops the internal failure without a message. Revive
runs the rules in random order, so deep-exit sometimes does not run
on args.go.

golangci-lint v2.15.0 uses revive v1.17.1, which logs the internal
failure and continues with the other rules.

This commit also makes these changes for v2.15.0:

- Disable exhaustruct_v5, as we already disable exhaustruct.
- Replace the deprecated gofumpt extra-rules setting with
  extra.group-params and extra.clothe-returns.
- Add a gosec nolint directive for the user-specified *_FILE path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
If a multi-line function call has its opening parenthesis at the end
of a line, balance-calls requires the closing parenthesis at the start
of a line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 26, 2026 21:24
@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: c13dc512-f126-439e-8dc0-541c67b293ed
Commits

Reviewing files that changed from the base of the PR and between 2157ff6 and 0c5f13b.

Files ignored due to path filters: 1
  • mise.lock is excluded by !**/*.lock
Files selected for processing: 6
  • .golangci.yml
  • cmd/geoipupdate/version.go
  • internal/errors.go
  • internal/geoipupdate/config.go
  • internal/geoipupdate/database/reader.go
  • internal/geoipupdate/geoip_updater_test.go
Files with no reviewable changes: 1
  • cmd/geoipupdate/version.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

The pull request updates linter and formatter configuration, removes a Go build constraint, and makes small changes to error extraction, JSON marshaling, a lint directive, and test fixture formatting.

Changes: Go maintenance

Layer / File(s) Summary
Lint settings and build selection
.golangci.yml, cmd/geoipupdate/version.go, internal/geoipupdate/config.go
The linter and formatter settings change. The go1.12 build constraint is removed, and a gosec suppression is added to a file-read call.
Error and JSON method updates
internal/errors.go, internal/geoipupdate/database/reader.go, internal/geoipupdate/geoip_updater_test.go
IsRetryableError uses errors.AsType[HTTPError]. ReadResult.MarshalJSON changes to a value receiver and no longer checks for a nil pointer. The test fixture formatting changes without changing its JSON payload.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: oschwald, marselester

Merge Risk: Minimal · up to 0c5f1

The PR is mergeable with no concrete merge-blocking risk identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: updating golangci-lint to v2.15.0 to address flaky revive results.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the linting lines,
Then hops past build tags, neat and fine.
A value receiver joins the play,
While errors take a clearer way.
The fixture rests, its bytes unchanged,
And carrots mark the code rearranged.

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Modver result

This report was generated by Modver,
a Go package and command that helps you obey semantic versioning rules in your Go module.

This PR requires (at least) an increase in your module's patchlevel.

no object *ReadResult.MarshalJSON in new version of package github.com/maxmind/geoipupdate/v8/internal/geoipupdate/database
  Patchlevel

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused tooling and compatibility changes have no identified blocking issues and align with the repository’s Go 1.26 baseline.

No open issues 🎉

What changed in this PR

Updates golangci-lint to v2.15.0 to address intermittent revive results, with configuration and Go code adjustments required by the upgrade.

Changes:

  • Updates lint tool versions, platform checksums, and configuration.
  • Modernizes error handling and JSON marshaling, and removes a redundant build constraint.
  • Enables gofumpt’s balanced-call formatting and reformats one test accordingly.
File Description
mise.lock Pins golangci-lint v2.15.0 across supported platforms.
internal/​geoipupdate/​geoip_updater_test.go Applies balanced-call formatting.
internal/​geoipupdate/​database/​reader.go Changes MarshalJSON to a value receiver.
internal/​geoipupdate/​config.go Documents a gosec suppression for user-specified file paths.
internal/​errors.go Uses errors.AsType for retryable HTTP errors.
cmd/​geoipupdate/​version.go Removes the redundant Go 1.12 build constraint.
.golangci.yml Updates linter exclusions and gofumpt settings.

Review effort: Balanced

@oschwald
oschwald merged commit 528b6d7 into main Oct 26, 2026
25 checks passed
@oschwald
oschwald deleted the wstorey/stf-2212-fix-flaky-golangci-lint-revive-results-in-geoipupdate branch October 26, 2026 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants