Skip to content

Bump the golangci-lint version#5434

Merged
Warashi merged 26 commits intomasterfrom
golangci-lint-gci
Dec 18, 2024
Merged

Bump the golangci-lint version#5434
Warashi merged 26 commits intomasterfrom
golangci-lint-gci

Conversation

@Warashi
Copy link
Member

@Warashi Warashi commented Dec 18, 2024

What this PR does:

  1. bump the golangci-lint version
  2. fix the failed checks

Why we need it:

I found the golangci-lint panics in the CI with logs below.

  /home/runner/work/_temp/reviewdog-YGym2u/golangci-lint-1.46.2-linux-amd64/golangci-lint run --out-format line-number
  panic: load embedded ruleguard rules: rules/rules.go:13: can't load fmt
  
  goroutine 1 [running]:
  github.com/go-critic/go-critic/checkers.init.22()
  	github.com/go-critic/go-critic@v0.6.3/checkers/embedded_rules.go:47 +0x4b4
  /home/runner/work/_temp/reviewdog-YGym2u/reviewdog -f=golangci-lint -name=golangci -reporter=github-pr-check -filter-mode=added -fail-on-error=false -level=error

ref; https://github.com/pipe-cd/pipecd/actions/runs/12371025156/job/34526255488

I ran the make lint/go in my local, I got the error below.

ERRO Running error: context loading failed: failed to load packages: failed to load with go/packages: err: exit status 1: stderr: go: go.mod requires go >= 1.22.4 (running go 1.21.3; GOTOOLCHAIN=local)

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@codecov
Copy link

codecov bot commented Dec 18, 2024

Codecov Report

Attention: Patch coverage is 39.02439% with 25 lines in your changes missing coverage. Please review.

Project coverage is 26.11%. Comparing base (40f8375) to head (b343897).
Report is 374 commits behind head on master.

Files with missing lines Patch % Lines
pkg/app/piped/livestatestore/ecs/store.go 0.00% 4 Missing ⚠️
pkg/oauth/oidc/oidc.go 55.55% 4 Missing ⚠️
pkg/app/piped/controller/controller.go 0.00% 3 Missing ⚠️
pkg/app/pipedv1/livestatestore/livestatestore.go 0.00% 3 Missing ⚠️
pkg/app/piped/controller/scheduler.go 0.00% 2 Missing ⚠️
pkg/app/piped/livestatestore/lambda/store.go 0.00% 2 Missing ⚠️
pkg/app/pipedv1/controller/scheduler.go 0.00% 2 Missing ⚠️
pkg/model/project.go 0.00% 2 Missing ⚠️
pkg/app/pipectl/cmd/initialize/initialize.go 0.00% 1 Missing ⚠️
pkg/app/pipedv1/cmd/piped/piped.go 0.00% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5434   +/-   ##
=======================================
  Coverage   26.10%   26.11%           
=======================================
  Files         451      451           
  Lines       48824    48823    -1     
=======================================
+ Hits        12747    12749    +2     
+ Misses      35059    35057    -2     
+ Partials     1018     1017    -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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
disable-all: true
enable:
- depguard
- exportloopref
Copy link
Member Author

Choose a reason for hiding this comment

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

golangci-lint tells me this rule is deprecated and not necessary.

copyloopvar is unnecessary because this rule tells us the copy of the loop variable, which we can delete.

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.

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
…consistency

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
…tions

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
…ompatibility

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
…nsistency

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi
Copy link
Member Author

Warashi commented Dec 18, 2024

There are no warnings/errors at the CI logs.
https://github.com/pipe-cd/pipecd/actions/runs/12385095982/job/34570765468?pr=5434

Running golangci-lint with reviewdog 🐶 ...
  /home/runner/work/_temp/reviewdog-Vf41uH/golangci-lint-1.62.2-linux-amd64/golangci-lint run --out-format line-number
  /home/runner/work/_temp/reviewdog-Vf41uH/reviewdog -f=golangci-lint -name=golangci -reporter=github-pr-check -filter-mode=added -fail-on-error=false -level=error

@Warashi Warashi marked this pull request as ready for review December 18, 2024 02:27
"sync/atomic"
"syscall"

"go.uber.org/atomic"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure this chance should be addressed by lint, wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think sync/atomic is enough, but we can use go.uber.org/atomic because it has more features than sync/atomic.

If we use sync/atomic, it's better not to mix sync/atomic and go.uber.org/atomic because it's confusing.

Currently, we have the rule below.

pipecd/.golangci.yml

Lines 44 to 45 in 40f8375

- pkg: "sync/atomic"
desc: "Use go.uber.org/atomic instead of sync/atomic."

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I got your point 👍

khanhtc1202
khanhtc1202 previously approved these changes Dec 18, 2024
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Great improvement 👍

@Warashi Warashi enabled auto-merge (squash) December 18, 2024 02:40
Copy link
Member

@ffjlabo ffjlabo left a comment

Choose a reason for hiding this comment

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

Nice work! I left a nit comment.

@@ -25,6 +25,7 @@ import (

Copy link
Member

Choose a reason for hiding this comment

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

nit: It would be nice to delete a line break between "golang.org/x/sync/singleflight" and "github.com/aws/aws-sdk-go-v2/service/lambda"

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!
Fixed this and similars on this commit.
b343897

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Copy link
Member

@ffjlabo ffjlabo left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

👍

@Warashi Warashi merged commit 7cf95df into master Dec 18, 2024
@Warashi Warashi deleted the golangci-lint-gci branch December 18, 2024 04:34
@github-actions github-actions bot mentioned this pull request Jan 21, 2025
@github-actions github-actions bot mentioned this pull request Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants