Feat/remove deprecated pre commit repo #290
Conversation
… new CI step to run golangci-lint using the github action
WalkthroughThis pull request introduces several modifications to the project's Go linting and pre-commit setups. A new GitHub Actions workflow for running golangci-lint has been added, triggered by pull requests and pushes on designated branches. In addition, the pre-commit workflow has been updated by bumping the checkout action version and omitting the installation steps for certain Go tools. The linting configuration file has been enhanced by enabling additional linters, while the pre-commit configuration for Go hooks has been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request/Push Event
participant GA as GitHub Actions
participant CO as Checkout Action
participant SG as Setup Go
participant GL as GolangCI-Lint
PR->>GA: Trigger workflow on push/PR event
GA->>CO: Checkout repository
CO-->>GA: Repository checked out
GA->>SG: Set up Go environment from go.mod
SG-->>GA: Go environment configured
GA->>GL: Execute golangci-lint with specified parameters
GL-->>GA: Return linting results
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/golangci-lint.yml(1 hunks).github/workflows/pre-commit.yml(1 hunks).golangci.yml(1 hunks).pre-commit-config.yaml(0 hunks)
💤 Files with no reviewable changes (1)
- .pre-commit-config.yaml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pre-commit.yml
11-11: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/golangci-lint.yml
16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
41-41: input "skip-pkg-cache" is not defined in action "golangci/golangci-lint-action@v6". available inputs are "args", "cache-invalidation-interval", "github-token", "install-mode", "only-new-issues", "problem-matchers", "skip-cache", "skip-save-cache", "version", "working-directory"
(action)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint
- GitHub Check: run-tests
- GitHub Check: pre-commit
🔇 Additional comments (2)
.golangci.yml (1)
12-13: LGTM! Good addition of formatting linters.The addition of
gofmtandgoimportslinters will help maintain consistent code formatting and import organization..github/workflows/golangci-lint.yml (1)
1-35: LGTM! Well-structured workflow configuration.The workflow is well-configured with appropriate permissions, triggers, and timeout settings. The verbosity flag will help in debugging any linting issues.
🧰 Tools
🪛 actionlint (1.7.4)
16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
| steps: | ||
| - uses: actions/setup-python@v4 | ||
| - uses: actions/checkout@master | ||
| - uses: actions/checkout@v3 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the checkout action version.
The actions/checkout@v3 action might be outdated. Consider updating to the latest version.
🌐 Web query:
What is the latest version of actions/checkout GitHub Action?
💡 Result:
The latest stable version of the actions/checkout GitHub Action is v4, as referenced across multiple sources[1][2][5][7]. This version addresses Node.js compatibility issues (deprecating Node.js 16 in favor of Node.js 20)[4][7] and includes reliability improvements for various environments[5].
For most workflows, you can implement it with:
- uses: actions/checkout@v4Key updates in recent versions:
- Compatibility with GitHub's updated runner environments
- Reduced dependency on Node.js tooling[5]
- Improved security practices for token handling[2]
Always check the official repository for new releases, as GitHub periodically updates actions to maintain compatibility with platform changes[6].
Citations:
- 1: https://graphite.dev/guides/github-actions-checkout
- 2: https://www.devzery.com/post/github-actions-complete-guide-to-actions-checkout
- 3: https://forum.gitea.com/t/use-actions-checkout-v3-in-a-container-without-node-installed/8406
- 4: https://discourse.julialang.org/t/how-do-i-update-github-actions-to-node-js-20/110536
- 5: https://github.com/marketplace/actions/checkout-action
- 6: https://github.com/actions/checkout
- 7: [Github Actions] actions/checkout@v3 should be upgraded to v4 denoland/fresh#2285
Action Required: Update GitHub Checkout Action Version
File: .github/workflows/pre-commit.yml (Line 11)
The workflow currently uses:
- uses: actions/checkout@v3Based on the latest information, the current stable version of the checkout action is v4. This update addresses Node.js compatibility (switching to Node.js 20), improves security practices, and enhances overall reliability. Please update the workflow step to:
- uses: actions/checkout@v4🧰 Tools
🪛 actionlint (1.7.4)
11-11: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/golangci-lint.yml
Outdated
| # skip-cache: true | ||
|
|
||
| # Optional: if set to true then the action don't cache or restore ~/go/pkg. | ||
| skip-pkg-cache: true |
There was a problem hiding this comment.
Remove unsupported cache parameter.
The skip-pkg-cache parameter is not supported by the golangci/golangci-lint-action@v6 action.
Remove this line as it's not a valid input parameter. If you need to skip caching, use the supported skip-cache: true parameter instead.
- skip-pkg-cache: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| skip-pkg-cache: true |
🧰 Tools
🪛 actionlint (1.7.4)
41-41: input "skip-pkg-cache" is not defined in action "golangci/golangci-lint-action@v6". available inputs are "args", "cache-invalidation-interval", "github-token", "install-mode", "only-new-issues", "problem-matchers", "skip-cache", "skip-save-cache", "version", "working-directory"
(action)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/golangci-lint.yml (1)
21-44: Consider enabling caching for better performance.Currently, all caching is disabled with
skip-cache: true. Enabling caching can significantly improve workflow execution time by reusing previous results.Consider applying this diff to enable caching:
# Optional: if set to true then the all caching functionality will be complete disabled, # takes precedence over all other caching options. # skip-cache: true # Optional: if set to true then the action don't cache or restore ~/go/pkg. - skip-cache: true + # skip-cache: true # Optional: if set to true then the action don't cache or restore ~/.cache/go-build. # skip-build-cache: true
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/golangci-lint.yml(1 hunks).golangci.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .golangci.yml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/golangci-lint.yml
16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint
🔇 Additional comments (2)
.github/workflows/golangci-lint.yml (2)
1-9: LGTM! Well-configured workflow triggers and permissions.The workflow is properly configured to run on pull requests and specific branch pushes, with appropriate minimal permissions.
11-14: LGTM! Appropriate job configuration.The job setup uses standard configuration with latest Ubuntu runner.
| - uses: actions/checkout@v3 | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version-file: "go.mod" | ||
| check-latest: true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update checkout action to latest version.
The actions/checkout action should be updated to v4 for better compatibility and security.
Apply this diff:
- - uses: actions/checkout@v3
+ - uses: actions/checkout@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v3 | |
| - uses: actions/setup-go@v5 | |
| with: | |
| go-version-file: "go.mod" | |
| check-latest: true | |
| - uses: actions/checkout@v4 | |
| - uses: actions/setup-go@v5 | |
| with: | |
| go-version-file: "go.mod" | |
| check-latest: true |
🧰 Tools
🪛 actionlint (1.7.4)
16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ethereum/util/noncecache.go (1)
131-136: Consider logging overflow events.The overflow handling is good, but consider logging when overflow occurs to help with monitoring and debugging.
if nonce > uint64(math.MaxInt64) { + log.Printf("warning: nonce overflow detected for account %s: %d > MaxInt64", account.String(), nonce) n.nonces[account] = math.MaxInt64 } else { n.nonces[account] = int64(nonce) }eip712_cosmos.go (1)
68-72: Consider extracting duplicate chainID validation.The chainID validation logic is duplicated in both WrapTxToEIP712 and WrapTxToEIP712V2.
Consider extracting it into a helper function:
+func validateAndConvertChainID(chainID uint64) (int64, error) { + if chainID > uint64(math.MaxInt64) { + return 0, fmt.Errorf("chainID is too large: %d (max supported value is %d)", chainID, math.MaxInt64) + } + return int64(chainID), nil +}Then use it in both functions:
-if chainID > uint64(math.MaxInt64) { - err := fmt.Errorf("chainID is too large: %d (max supported value is %d)", chainID, math.MaxInt64) - return typeddata.TypedData{}, err -} -intChainId := int64(chainID) +intChainId, err := validateAndConvertChainID(chainID) +if err != nil { + return typeddata.TypedData{}, err +}Also applies to: 542-546
🧰 Tools
🪛 GitHub Check: lint
[failure] 69-69:
printf: fmt.Errorf format %s has arg chainID of wrong type uint64 (govet)🪛 GitHub Actions: golangci-lint
[error] 69-69: printf: fmt.Errorf format %s has arg chainID of wrong type uint64 (govet)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
eip712_cosmos.go(5 hunks)ethereum/util/noncecache.go(2 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
eip712_cosmos.go
[failure] 69-69:
printf: fmt.Errorf format %s has arg chainID of wrong type uint64 (govet)
[failure] 543-543:
printf: fmt.Errorf format %s has arg chainID of wrong type uint64 (govet)
🪛 GitHub Actions: golangci-lint
eip712_cosmos.go
[error] 69-69: printf: fmt.Errorf format %s has arg chainID of wrong type uint64 (govet)
| if chainID > uint64(math.MaxInt64) { | ||
| err := fmt.Errorf("chainID is too large: %s (max supported value is %s)", chainID, math.MaxInt64) | ||
| return typeddata.TypedData{}, err | ||
| } |
There was a problem hiding this comment.
Fix formatting in error messages.
The error messages use incorrect format specifier for uint64 values.
Apply this fix to both error messages:
-err := fmt.Errorf("chainID is too large: %s (max supported value is %s)", chainID, math.MaxInt64)
+err := fmt.Errorf("chainID is too large: %d (max supported value is %d)", chainID, math.MaxInt64)Also applies to: 542-545
🧰 Tools
🪛 GitHub Check: lint
[failure] 69-69:
printf: fmt.Errorf format %s has arg chainID of wrong type uint64 (govet)
🪛 GitHub Actions: golangci-lint
[error] 69-69: printf: fmt.Errorf format %s has arg chainID of wrong type uint64 (govet)
Summary by CodeRabbit
New Features
gofmtandgoimports, improving code formatting and import validation.skip-dirsoption in the linting configuration to exclude specific directories from checks.Chores