chore: upgrade to Go 1.23.1 #3848
Conversation
rootulp
requested review from
rach-id,
evan-forbes,
liamsi and
a team
as code owners
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request includes updates to various configuration files and dependencies in the project. Key changes involve upgrading the versions of tools such as Changes
Possibly related PRs
Suggested labels
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
Documentation and Community
|
rootulp
requested review from
cmwaters and
ninabarbakadze
and removed request for
liamsi
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
tools/blocketa/README.md (1)
18-18: Add a newline at the end of the file.The static analysis tool suggests adding a single newline character at the end of the file. This is a good practice for file formatting.
> The block time is currently hard-coded. If you're running this for a network with a different block time, you'll need to update the `blockTime` constant in the main.go file. You can use [https://www.mintscan.io/celestia/block](https://www.mintscan.io/celestia/block/) or the blocktime tool. +Tools
Markdownlint
18-18: null
Files should end with a single newline character(MD047, single-trailing-newline)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- tools/blocketa/README.md (1 hunks)
Additional context used
Markdownlint
tools/blocketa/README.md
18-18: null
Files should end with a single newline character(MD047, single-trailing-newline)
Additional comments not posted (1)
tools/blocketa/README.md (1)
18-18: LGTM!The change to the URL formatting improves the readability of the documentation.
Tools
Markdownlint
18-18: null
Files should end with a single newline character(MD047, single-trailing-newline)
Pull request was converted to draft
|
There's a bug in Go 1.23 golang/go#69312 that is impacting celestia-node so let's hold off on merging this until we identify if the same issue may hit us. |
Co-authored-by: CHAMI Rachid <[email protected]>
Co-authored-by: CHAMI Rachid <[email protected]>
Co-authored-by: CHAMI Rachid <[email protected]>
Co-authored-by: CHAMI Rachid <[email protected]>
Co-authored-by: CHAMI Rachid <[email protected]>
|
Looks like golang/go#69312 was fixed in Go 1.23.2. celestia-app doesn't invoke |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
README.md (1)
Line range hint
77-95: BBR congestion control instructions are a great addition.The new instructions for enabling the BBR congestion control algorithm are clear and helpful. This addition will improve network performance for users.
Consider adding a note about system restart requirements (if any) after applying these changes.
go.mod (3)
3-3: Approve Go version update, but consider 1.23.2The update to Go 1.23.1 is good, but since Go 1.23.2 is available and resolves the bug mentioned in issue #69312, consider upgrading to 1.23.2 for additional bug fixes and improvements.
Would you like to update to Go 1.23.2 in this PR?
Line range hint
5-184: Approve dependency updates, suggest documenting pinned versionThe dependency updates look good and seem to be part of routine maintenance. However, it's worth noting that
github.com/cosmos/cosmos-sdkhas been pinned tov0.46.16.Consider adding a comment in the
go.modfile or in the project documentation explaining why this specific version ofcosmos-sdkis pinned. This will help future maintainers understand the reasoning behind this decision.
Line range hint
186-194: Approve replace directives, suggest documenting fork usageThe replace directives look good and seem to address specific project needs. The comment explaining the pin for
ledger-cosmos-gois helpful.Consider adding similar comments for the other replace directives, especially for the forks of
cosmos-sdkandtendermint. This will help future maintainers understand why these forks are being used instead of the original repositories.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- .github/workflows/lint.yml (1 hunks)
- Makefile (2 hunks)
- README.md (2 hunks)
- docker/Dockerfile (1 hunks)
- docker/txsim/Dockerfile (1 hunks)
- go.mod (1 hunks)
🔇 Additional comments (9)
.github/workflows/lint.yml (1)
25-25: Approved: golangci-lint-action version updateThe update to golangci-lint-action version v1.61.0 is appropriate and aligns with the PR objectives. This keeps the CI tools up-to-date, which is a good practice.
Reminder: As mentioned in the PR description, maintainers should update their local
golangci-lintbinary to version 1.61.0. You can verify the installed version locally using:If an update is needed, follow the installation instructions provided in the PR description.
docker/txsim/Dockerfile (3)
Line range hint
28-28: Approve Alpine Linux version update in the second stage.Updating to
alpine:3.20in the second stage maintains consistency with the builder stage and keeps the final image up-to-date. This is a good practice for security and performance reasons.
Line range hint
1-64: Summary of Dockerfile changes and their implications.The Dockerfile has been successfully updated to use Go 1.23.1 and Alpine 3.20 in both stages. These changes align with the PR objective and represent good practices in keeping the build environment up-to-date.
Key points:
- The updates are consistent across both stages of the build.
- No other changes were required, suggesting good compatibility with the new versions.
- The minimal nature of the changes reduces the risk of introducing new issues.
Recommendations:
- Ensure thorough testing of the built image to verify compatibility with Go 1.23.1.
- Update any relevant documentation or CI/CD pipelines to reflect the new Go version requirement.
- Monitor for any performance changes or new warnings/errors during the build process.
To ensure the update doesn't introduce any regressions, please run your full test suite with the new image. Additionally, consider comparing the size and performance of the final image:
#!/bin/bash # Build both old and new images docker build -t txsim-old -f docker/txsim/Dockerfile.old . docker build -t txsim-new -f docker/txsim/Dockerfile . # Compare image sizes echo "Old image size:" docker images txsim-old --format "{{.Size}}" echo "New image size:" docker images txsim-new --format "{{.Size}}" # Compare startup times (if applicable) time docker run --rm txsim-old txsim version time docker run --rm txsim-new txsim version
2-2: Approve Go and Alpine Linux version update.The update to
golang:1.23.1-alpine3.20aligns with the PR objective of upgrading to Go 1.23.1. The Alpine Linux update to 3.20 is also a good practice for keeping the base image current.To ensure compatibility, please verify that the application builds and runs correctly with these updated versions. Consider running the following commands in your CI pipeline or locally:
docker/Dockerfile (1)
7-7: Approved: Go version upgrade looks good.The update from
golang:1.22.6-alpine3.19togolang:1.23.1-alpine3.20aligns with the PR objective of upgrading to Go 1.23.1. This change should bring the latest improvements and bug fixes from Go 1.23.1.Consider the following points:
- Ensure all project dependencies are compatible with Go 1.23.1.
- The Alpine version has also been updated from 3.19 to 3.20. Verify that this doesn't introduce any unexpected changes in the build environment.
- Confirm that the build process succeeds with this new image.
- Update any relevant documentation to reflect this Go version change.
- Ensure that CI/CD pipelines are updated to use Go 1.23.1.
To verify the Go version and Alpine version, run:
README.md (3)
38-38: Go version update looks good.The Go version has been correctly updated to 1.23.1, which aligns with the PR objective.
Line range hint
127-131: Improved Contributing guidelines.The added emphasis on conventional commits and the instructions for using
go.workare valuable additions. These changes will help maintain consistent commit messages and improve the development workflow for contributors working with multiple Go modules.
Line range hint
168-172: Audit information updated appropriately.The addition of the new audit entry for Informal Systems (dated 2024/7/1) keeps the security information current and demonstrates ongoing commitment to the project's security. The format is consistent with previous entries.
Makefile (1)
10-12: LGTM! Verify impact on other parts of the project.The upgrade to Go 1.23.1 is implemented as intended. The added comment about verifying the Docker image existence is a helpful reminder for maintainers.
To ensure this change doesn't introduce any incompatibilities, please run the following script to check for any hardcoded Go version references in the project:
✅ Verification successful
LGTM! The Go version has been successfully upgraded to 1.23.1 without any lingering references to the old version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for hardcoded Go version references that might need updating # Search for Go version references in all files echo "Searching for Go version references:" rg --type-add 'go:*.{go,mod}' --type-add 'yaml:*.{yml,yaml}' --type go --type yaml --type make '1\.22|go1\.22' . # Check go.mod file for Go version echo -e "\nChecking go.mod file:" if [ -f go.mod ]; then grep "^go " go.mod else echo "go.mod file not found" fi # Check GitHub Actions workflows for Go setup echo -e "\nChecking GitHub Actions workflows:" rg --type yaml 'uses: actions/setup-go@v[0-9]+' .github/workflowsLength of output: 1151
Description
Upgrade to Go 1.23.1 motivated by #3847. I had to modify GoReleaser config because goreleaser/goreleaser#5127.
Maintainers need to update their local golangic-lint binary to v1.61.0. See golangci-lint install.