-
Notifications
You must be signed in to change notification settings - Fork 238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Problem: cometbft not up to date #1664
Conversation
update to 0.38.13
WalkthroughThe changes in this pull request include updates to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
CodeRabbit Configuration File (
|
Signed-off-by: yihuang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: yihuang <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1664 +/- ##
=======================================
Coverage 34.99% 34.99%
=======================================
Files 123 123
Lines 11783 11783
=======================================
Hits 4124 4124
Misses 7245 7245
Partials 414 414 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
nix/golangci-lint.nix (1)
14-26
: Consider using a dynamic build date.The build configuration looks good overall, but the hardcoded date (19700101-00:00:00) could be replaced with the actual build date for better debugging information.
Consider updating the date flag:
- "-X main.date=19700101-00:00:00" + "-X main.date=$(date -u +%Y%m%d-%H:%M:%S)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- docs/docs.go (1 hunks)
- nix/build_overlay.nix (1 hunks)
- nix/golangci-lint.nix (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/docs.go
🔇 Additional comments (6)
nix/build_overlay.nix (2)
4-4
: LGTM! The golangci-lint package addition follows Nix conventions.The syntax is correct and consistent with the existing pattern used for other packages in this overlay.
4-4
: Verify the relationship with cometbft update.The addition of golangci-lint appears to be separate from the main PR objective of updating cometbft. Please clarify if this is:
- A required tooling update to support the newer cometbft version
- An unrelated improvement that should be in a separate PR
✅ Verification successful
Let me gather more information about the relationship between golangci-lint and cometbft.
Let me check one more thing to see if golangci-lint is used in any Go-related build or test processes.
golangci-lint is a required development dependency
The verification shows that golangci-lint is an essential development tool used in the project's build process:
- It's integrated into the main Makefile with dedicated lint targets (
lint
,lint-fix
)- The project has a
.golangci.yml
configuration file- It's used for Go code linting as part of the development workflow
This is a required tooling dependency that belongs in this PR as it's needed for the Go-based development workflow, which includes working with CometBFT.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if golangci-lint is mentioned in cometbft's build requirements rg -l "golangci-lint" --type nixLength of output: 76
Script:
#!/bin/bash # Check the content of golangci-lint.nix to understand its purpose cat nix/golangci-lint.nix # Search for any direct relationship between cometbft and golangci-lint rg -i "cometbft.*golangci-lint|golangci-lint.*cometbft" -A 5 -B 5 # Check if there are any recent changes or commits mentioning both git log --grep="cometbft.*golangci|golangci.*cometbft" --allLength of output: 1315
Script:
#!/bin/bash # Check for any Go-related build or test files that might use golangci-lint rg -l "\.go" --type yaml --type nix # Look for any Makefile or similar build scripts fd Makefile # Check content of any found build files rg "lint" --type yaml --type makeLength of output: 1223
nix/golangci-lint.nix (4)
7-12
: LGTM! Secure source configuration.The source fetching configuration is well-defined with proper hash verification for security.
28-33
: LGTM! Comprehensive shell completion setup.The post-installation script properly generates and installs shell completions for all major shells, with appropriate security measures using TMPDIR.
35-42
:⚠️ Potential issueFix incorrect license specification.
The license is specified as GPL3+, but golangci-lint uses the MIT license according to their repository.
Update the license specification:
- license = licenses.gpl3Plus; + license = licenses.mit;Let's verify the license:
#!/bin/bash # Description: Check the license in golangci-lint repository gh api repos/golangci/golangci-lint --jq .license.spdx_id
1-6
: Verify the builder name and version number.Two potential issues to address:
- The builder name
buildGo123Module
seems unusual. Should this bebuildGoModule
instead?- The specified version 1.60.1 appears to be incorrect as it's higher than the latest released version (1.55.2) of golangci-lint.
Let's verify the latest version:
update to 0.38.13
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make
)make test
)go fmt
)golangci-lint run
)go list -json -m all | nancy sleuth
)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit
Documentation
CHANGELOG.md
to include a new section titled "UNRELEASED" for tracking improvements.Chores
flake.nix
to reference a stable release ofnixpkgs
.nix/sources.json
to reflect changes in thenixpkgs
package source.golangci-lint
package to the build overlay for enhanced linting capabilities.golangci-lint
tool, facilitating easier linting integration.