Fix all lint issues #981

lukasschwab · 2025-03-14T00:43:23Z

Changes

26385d9 Fix all lint issues. Makes minimal changes — mostly renaming unused parameters to _ for revive — to bring all files into compliance with the current golangci-lint config.
- Exempts http/example_test.go from linting — the lint errors are valid, but this is example code and I want to avoid changing public-facing documentation in this PR. Consider removing the //nolint directive in a follow-up.
48a9b1d Update lint.yml; allow secretless linting of PRs from public forks.
- See e.g. Expose MockTransport, MockScope in root sentry package #972 (comment))
- Remove the only-new-issues: true and github-token options from the golangci-lint-action step. github-token is only necessary when only-new-issues: true.
- Remove the "Get auth token" step: only necessary to provide github-token to the golangci-lint-action step.

Testing

I believe CI should run the version of the lint.yml action on this branch.

make test and make lint succeed locally:

$ make test lint 
>>> Running tests for module: .
(cd . && go test -count=1 -timeout 300s  ./...)
?       github.com/getsentry/sentry-go/internal/debug   [no test files]
?       github.com/getsentry/sentry-go/internal/otel/baggage/internal/baggage   [no test files]
?       github.com/getsentry/sentry-go/internal/testutils       [no test files]
ok      github.com/getsentry/sentry-go  2.297s
ok      github.com/getsentry/sentry-go/http     0.427s
ok      github.com/getsentry/sentry-go/internal/httputils       0.712s
ok      github.com/getsentry/sentry-go/internal/otel/baggage    0.872s
ok      github.com/getsentry/sentry-go/internal/ratelimit       1.080s
ok      github.com/getsentry/sentry-go/internal/traceutils      1.233s
>>> Running tests for module: ./echo
(cd ./echo && go test -count=1 -timeout 300s  ./...)
ok      github.com/getsentry/sentry-go/echo     0.592s
>>> Running tests for module: ./fasthttp
(cd ./fasthttp && go test -count=1 -timeout 300s  ./...)
ok      github.com/getsentry/sentry-go/fasthttp 0.513s
>>> Running tests for module: ./fiber
(cd ./fiber && go test -count=1 -timeout 300s  ./...)
ok      github.com/getsentry/sentry-go/fiber    0.532s
>>> Running tests for module: ./gin
(cd ./gin && go test -count=1 -timeout 300s  ./...)
ok      github.com/getsentry/sentry-go/gin      0.505s
>>> Running tests for module: ./iris
(cd ./iris && go test -count=1 -timeout 300s  ./...)
ok      github.com/getsentry/sentry-go/iris     0.429s
>>> Running tests for module: ./logrus
(cd ./logrus && go test -count=1 -timeout 300s  ./...)
ok      github.com/getsentry/sentry-go/logrus   0.495s
>>> Running tests for module: ./negroni
(cd ./negroni && go test -count=1 -timeout 300s  ./...)
ok      github.com/getsentry/sentry-go/negroni  0.487s
>>> Running tests for module: ./otel
(cd ./otel && go test -count=1 -timeout 300s  ./...)
ok      github.com/getsentry/sentry-go/otel     0.437s
ok      github.com/getsentry/sentry-go/otel/internal/utils      0.474s
>>> Running tests for module: ./slog
(cd ./slog && go test -count=1 -timeout 300s  ./...)
ok      github.com/getsentry/sentry-go/slog     0.356s
>>> Running tests for module: ./zerolog
(cd ./zerolog && go test -count=1 -timeout 300s  ./...)
ok      github.com/getsentry/sentry-go/zerolog  0.565s
golangci-lint run
WARN The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar.

``` $ golangci-lint run ./... --max-issues-per-linter=0 --max-same-issues=0 ```

lukasschwab · 2025-03-14T01:10:04Z

hub.go

-	max := client.options.MaxBreadcrumbs
+	limit := client.options.MaxBreadcrumbs
 	switch {
-	case max < 0:
+	case limit < 0:
 		return
-	case max == 0:
-		max = defaultMaxBreadcrumbs
-	case max > maxBreadcrumbs:
-		max = maxBreadcrumbs


limit is the name used by AddBreadcrumb, and it doesn't shadow the Go builtin max.

codecov · 2025-03-14T18:08:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.65%. Comparing base (cfbfc6b) to head (6c834d4).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #981      +/-   ##
==========================================
+ Coverage   83.64%   83.65%   +0.01%     
==========================================
  Files          50       50              
  Lines        5159     5159              
==========================================
+ Hits         4315     4316       +1     
  Misses        687      687              
+ Partials      157      156       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

lukasschwab · 2025-03-14T18:51:09Z

@cleptric this PR is of course partially redundant with #982. This begs the question — is it necessary to remove the only-new-issues: true check?

For reference, here's the golangci-lint-action logic for fetching the PR patch:

https://github.com/golangci/golangci-lint-action/blob/4696ba8babb6127d732c3c6dde519db15edab9ea/src/patch.ts#L40-L82

Without the token provided, I'd expect octokit.rest.pulls.get(...) to fail. But I don't see "failed to fetch pull request patch" in your lint run logs for that PR.

I think this is because golangci-lint-action defaults to ${{ github.token }} when a token isn't explicitly provided:

https://github.com/golangci/golangci-lint-action/blob/4696ba8babb6127d732c3c6dde519db15edab9ea/action.yml#L23

That's fine for PRs from branches in this repo, but I believe that secret will not be set for PRs from external forks. If the secret isn't set, golangci-lint logs a warning and disregards the only-new-issues option entirely.

It seems preferable to just drop the flag. If there are no lint issues on master, then any lint issue on a PR is, by definition, new :)

cleptric · 2025-03-14T20:00:49Z

Yes, we can just remove it and lint should fail on all, not just new issues. Will review this next week.

cleptric

Thanks!

lukasschwab added 2 commits March 13, 2025 17:30

Fix all lint issues

26385d9

``` $ golangci-lint run ./... --max-issues-per-linter=0 --max-same-issues=0 ```

Remove GitHub token, only-new-issues from lint.yml workflow

48a9b1d

lukasschwab commented Mar 14, 2025

View reviewed changes

lukasschwab mentioned this pull request Mar 14, 2025

Expose MockTransport, MockScope in root sentry package #972

Merged

Merge remote-tracking branch 'origin/master' into ls/fix-lint-issues

6c834d4

lukasschwab changed the title ~~Fix lint issues~~ Fix all lint issues Mar 14, 2025

cleptric self-requested a review March 14, 2025 20:09

cleptric self-assigned this Mar 14, 2025

cleptric added the Type: Improvement label Mar 14, 2025

cleptric approved these changes Mar 17, 2025

View reviewed changes

cleptric merged commit 6019770 into getsentry:master Mar 17, 2025
16 checks passed

lukasschwab deleted the ls/fix-lint-issues branch March 17, 2025 16:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix all lint issues #981

Fix all lint issues #981

Uh oh!

lukasschwab commented Mar 14, 2025 •

edited

Loading

Uh oh!

lukasschwab Mar 14, 2025 •

edited

Loading

Uh oh!

codecov bot commented Mar 14, 2025 •

edited

Loading

Uh oh!

lukasschwab commented Mar 14, 2025 •

edited

Loading

Uh oh!

cleptric commented Mar 14, 2025

Uh oh!

cleptric left a comment

Uh oh!

Uh oh!

Uh oh!

Fix all lint issues #981

Fix all lint issues #981

Uh oh!

Conversation

lukasschwab commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Testing

Uh oh!

lukasschwab Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lukasschwab commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cleptric commented Mar 14, 2025

Uh oh!

cleptric left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lukasschwab commented Mar 14, 2025 •

edited

Loading

lukasschwab Mar 14, 2025 •

edited

Loading

codecov bot commented Mar 14, 2025 •

edited

Loading

lukasschwab commented Mar 14, 2025 •

edited

Loading