Chore/go 1.23 #779
Chore/go 1.23 #779
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## main #779 +/- ##
===========================================
- Coverage 55.42% 41.79% -13.63%
===========================================
Files 73 108 +35
Lines 2966 5024 +2058
===========================================
+ Hits 1644 2100 +456
- Misses 1228 2813 +1585
- Partials 94 111 +17 |
WalkthroughThe pull request updates various workflow configurations and code files to transition the Go version from Changes
Possibly related PRs
Suggested reviewers
Poem
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
.github/workflows/test-e2e.yml (2)
Line range hint
22-23: Consider using major version references for actions.While using fixed versions (e.g.,
v5.0.2) ensures stability, consider using major version references (e.g.,v5) for actions. This allows you to automatically receive minor updates and patches while maintaining compatibility.For example:
- name: Setup Go environment uses: actions/setup-go@v5 with: go-version: "1.23"This approach balances stability with receiving important updates.
Line range hint
43-52: Address TODO comment in "Clean cluster" step.There's a TODO comment suggesting a change to the cleanup step:
# todo(@anmol1696): change this to be post step of the actionConsider implementing this improvement or creating an issue to track this task. Moving the cleanup to a post-action step could potentially improve the workflow's robustness and clarity.
Would you like me to help draft a GitHub issue for tracking this TODO item?
.github/workflows/test.yml (1)
Line range hint
61-77: Approved: Go version updated and conditional execution added.The
test-blockchainjob has been successfully updated to use Go version 1.23, aligning with the PR objective. The job now runs conditionally based on thecheck-testsoutput, which is an excellent optimization.Consider enhancing the blockchain startup verification step. Currently, it only checks for the presence of the "address" field in the validator info. A more robust check could include verifying the blockchain's height or checking for a specific number of validators. Here's a suggested improvement:
- name: Verify blockchain startup run: | STATUS=$(curl http://0.0.0.0:26657/status) - CHECK=$(echo $STATUS | jq '.result.validator_info | has("address")') + ADDRESS_CHECK=$(echo $STATUS | jq '.result.validator_info | has("address")') + HEIGHT_CHECK=$(echo $STATUS | jq '.result.sync_info.latest_block_height | tonumber > 0') - if [ $CHECK != "true" ]; then + if [ "$ADDRESS_CHECK" != "true" ] || [ "$HEIGHT_CHECK" != "true" ]; then echo "❌ Blockchain test failed" echo "$STATUS" exit -1 fi + echo "✅ Blockchain started successfully with valid height"This change ensures that not only the validator address is present, but also that the blockchain has produced at least one block.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (14)
- .github/workflows/build.yml (1 hunks)
- .github/workflows/lint.yml (3 hunks)
- .github/workflows/test-e2e.yml (1 hunks)
- .github/workflows/test.yml (2 hunks)
- .golangci.yml (1 hunks)
- Dockerfile (3 hunks)
- Makefile (2 hunks)
- README.md (1 hunks)
- cmd/axoned/cmd/genaccount_test.go (0 hunks)
- go.mod (1 hunks)
- x/mint/client/cli/query_test.go (0 hunks)
- x/mint/keeper/msg_server_test.go (0 hunks)
- x/vesting/client/cli/tx_test.go (0 hunks)
- x/vesting/types/vesting_account_test.go (0 hunks)
💤 Files with no reviewable changes (5)
- cmd/axoned/cmd/genaccount_test.go
- x/mint/client/cli/query_test.go
- x/mint/keeper/msg_server_test.go
- x/vesting/client/cli/tx_test.go
- x/vesting/types/vesting_account_test.go
🧰 Additional context used
🔇 Additional comments (22)
Dockerfile (4)
Line range hint
1-28: Overall: Good updates, verify build and runtime behaviorThe Dockerfile changes align well with the PR objectives, updating to Go 1.23 and related components. The updates to base images and package versions follow good practices for maintaining security and compatibility.
Final recommendations:
- Ensure all CI/CD pipelines complete successfully with these changes.
- Verify that the final Docker image size hasn't increased significantly.
- Conduct thorough testing of the application's runtime behavior in the new environment.
- Update any documentation that references the Go version or Alpine version if necessary.
2-2: LGTM: Base image updated to Go 1.23The update to
golang:1.23-alpine3.20aligns with the PR objective of upgrading to Go 1.23. This change also includes an update to the Alpine version, which is a good practice for keeping the base image current.To ensure the build process still works as expected with the new base image, please verify the following:
- The application builds successfully in the CI/CD pipeline.
- All tests pass with the new Go version.
- There are no unexpected changes in the build artifacts.
#!/bin/bash # Description: Verify Go version and successful build # Test: Check if Go 1.23 is used in the CI/CD pipeline gh run list --limit 1 --json jobs --jq '.[] | .jobs[] | select(.name == "build") | .steps[] | select(.name == "Set up Go") | .env.go-version' # Test: Verify successful build in the latest CI run gh run list --limit 1 --json conclusion --jq '.[].conclusion'
21-21: LGTM: Final stage base image updatedThe update to
alpine:3.20.3aligns with the build stage Alpine version update. This is a good practice for maintaining consistency and potentially benefiting from security updates and optimizations in the newer Alpine version.To ensure the application runs correctly with the new base image:
- Verify that the application starts and runs as expected in the new environment.
- Check for any runtime dependencies that might be affected by the Alpine version change.
- Ensure that the image size hasn't increased significantly.
12-12: LGTM: Package versions updatedThe updates to package versions are good practice for maintaining security and compatibility:
- ca-certificates: 20240226-r0 -> 20240705-r0
- git: ~2.40 -> ~2.45
- linux-headers: 6.3-r0 -> 6.6-r0
These updates align well with the newer Alpine version and Go update.
To ensure these package updates don't introduce any unexpected issues:
- Verify that all build steps complete successfully with these new package versions.
- Check if there are any deprecation warnings or new features in the git changelog between 2.40 and 2.45 that might affect your build process.
.github/workflows/build.yml (2)
26-26: Go version update aligns with PR objectives.The change from Go version '1.21' to '1.23' in the 'Setup Go environment' step is consistent with the PR objective to update the project to utilize Go version 1.23. This update ensures that the build process uses the latest Go version as intended.
To ensure consistency across the project, let's verify if other files or workflow steps need similar updates:
#!/bin/bash # Description: Check for any remaining references to Go 1.21 or 1.22 in workflow files and go.mod echo "Checking workflow files for Go version references:" rg --type yaml '(go-version:|GO_VERSION=).*1\.(21|22)' .github/workflows/ echo "Checking go.mod for Go version:" cat go.mod | grep "^go 1." echo "Checking Dockerfile for Go version:" rg --type dockerfile '(FROM golang:|GO_VERSION=).*1\.(21|22)' Dockerfile
Line range hint
49-49: Consider updating the 'tj-actions/changed-files' action.The 'build-proto' job uses 'tj-actions/changed-files@v45.0.3'. Consider updating this action to the latest version for potential improvements and bug fixes.
Let's check for the latest version of this action:
✅ Verification successful
'tj-actions/changed-files' action is up to date.
The current version
v45.0.3is the latest available release.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the latest version of tj-actions/changed-files action echo "Latest version of tj-actions/changed-files:" gh release list --repo tj-actions/changed-files --limit 1Length of output: 383
.github/workflows/test-e2e.yml (2)
27-27: Go version update looks good.The Go version has been correctly updated from 1.21 to 1.23, which aligns with the PR objective.
Line range hint
3-8: Verify intention of commented workflow triggers.The workflow triggers for both push and pull requests are currently commented out. This means the workflow won't run automatically on code changes. Is this intentional? If not, consider uncommenting these lines to ensure the E2E tests run on relevant events.
.golangci.yml (2)
10-10: LGTM! Appropriate linter update for Go 1.23The addition of
copyloopvarand the implicit removal ofexportlooprefalign perfectly with the PR objectives and the transition to Go 1.23. This change is appropriate because:
copyloopvaris the recommended replacement forexportlooprefin Go 1.22 and later versions.exportlooprefis no longer necessary since Go 1.22, as the language now handles loop variable capture correctly.This update ensures that the project maintains robust linting while staying current with the latest Go version.
Line range hint
1-135: Configuration integrity maintainedThe rest of the
.golangci.ymlfile remains unchanged, which is appropriate given the scope of this PR. The existing configuration is comprehensive and well-structured, covering various aspects of code quality checks. This consistency in the rest of the file ensures that:
- Other important linters continue to run as before.
- Linter settings, output configurations, and issue exclusion rules are preserved.
Maintaining this configuration while updating the Go version and specific linters demonstrates a balanced approach to keeping the project up-to-date without introducing unnecessary changes.
.github/workflows/test.yml (3)
Line range hint
16-30: Great addition of thecheck-testsjob!This new job efficiently determines if any test-related files have been modified. By outputting a status based on detected changes, it allows subsequent jobs to run conditionally. This optimization can significantly reduce unnecessary test runs, saving time and resources in the CI pipeline.
Line range hint
1-114: Summary: Workflow successfully updated to Go 1.23 with optimizationsThis PR successfully updates the GitHub Actions workflow to use Go version 1.23 across all jobs, meeting the primary objective. The addition of the
check-testsjob and conditional execution of subsequent jobs is an excellent optimization that can significantly reduce unnecessary test runs.Key improvements:
- Go version updated to 1.23 in both
test-goandtest-blockchainjobs.- New
check-testsjob added to detect changes in relevant files.- Conditional execution of test jobs based on
check-testsoutput.These changes should result in faster CI runs for PRs that don't modify test-related files, while ensuring thorough testing when necessary. Great work on improving the efficiency of the CI pipeline!
Line range hint
32-46: Approved: Go version updated and conditional execution added.The
test-gojob has been successfully updated to use Go version 1.23, aligning with the PR objective. The job now runs conditionally based on thecheck-testsoutput, which is an excellent optimization.To ensure the Go version is consistently updated across the project, let's run the following check:
✅ Verification successful
Verification Successful: Go version is consistently set to 1.23 across the project.
All configuration files, including
go.mod,Dockerfile, and GitHub workflows, are correctly using Go version 1.23.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Go version is consistently set to 1.23 across the project # Check Go version in go.mod echo "Checking go.mod:" grep -n "^go 1\.23$" go.mod || echo "go.mod does not specify Go 1.23" # Check Go version in Dockerfile (if exists) echo -e "\nChecking Dockerfile:" grep -n "FROM golang:1\.23" Dockerfile || echo "Dockerfile not found or does not use Go 1.23" # Check Go version in other configuration files echo -e "\nChecking other configuration files:" grep -r -n "go-version.*1\.23" .githubLength of output: 773
.github/workflows/lint.yml (4)
146-146: LGTM: Consistent Go version updateThe Go version update to 1.23 in the
analyze-gojob maintains consistency with thelint-gojob.
201-201: LGTM: Go 1.23 update completedThe Go version update to 1.23 in the
lint-generatedjob completes the consistent update across all relevant jobs in the workflow.This change, along with the previous updates, successfully implements the PR objective of upgrading to Go 1.23 throughout the lint workflow.
Line range hint
1-238: Missing changes from PR objectivesWhile the Go version has been successfully updated to 1.23, there are two important changes mentioned in the PR objectives that are not reflected in this file:
- Updating gofumpt to v0.7.0
- Replacing the exportloopref linter with copyloopvar
Please implement these changes to fully meet the PR objectives:
- Update the gofumpt installation command in the
lint-gojob:- go install mvdan.cc/gofumpt@v0.4.0 + go install mvdan.cc/gofumpt@v0.7.0
- Update the golangci-lint configuration to replace exportloopref with copyloopvar. This change should be made in the
.golangci.ymlfile, which is not present in this diff. Please ensure this change is made in the appropriate file.To verify the gofumpt version after the change:
#!/bin/bash # Description: Verify gofumpt version in the lint-go job # Test: Search for gofumpt installation. Expect: v0.7.0 rg --type yaml 'go install mvdan.cc/gofumpt@' .github/workflows/lint.ymlTo verify the linter change:
#!/bin/bash # Description: Verify linter configuration in .golangci.yml # Test: Search for copyloopvar in linter config. Expect: copyloopvar present, exportloopref absent rg --type yaml 'linters:' .golangci.yml -A 10
102-102: LGTM: Go version updated to 1.23This change aligns with the PR objective to update to Go 1.23.
Let's verify that this version is consistent across the workflow:
✅ Verification successful
Re-run Go version consistency check with corrected file path
Please execute the following script to verify that all
go-versionentries in.github/workflows/lint.ymlare set to"1.23":
Verified: Go version consistency confirmed
All instances of
go-versionin.github/workflows/lint.ymlare set to"1.23".🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistency of Go version across the workflow file # Test: Search for all occurrences of go-version. Expect: All should be "1.23" rg --type yaml 'go-version:' .github/workflows/lint.ymlLength of output: 246
Script:
#!/bin/bash # Description: Check for consistency of Go version across the workflow file # Test: Search for all occurrences of go-version. Expect: All should be "1.23" rg --type yaml 'go-version: "1.23"' .github/workflows/lint.ymlLength of output: 151
go.mod (3)
Line range hint
5-77: Dependency updates approved.The updates to various dependencies likely bring bug fixes, performance improvements, and new features to the project. New dependencies have also been added, which may introduce new functionality or replace deprecated ones.
Please ensure that these dependency updates don't introduce any breaking changes to the project. Run the following script to check for any potential issues:
#!/bin/bash # Description: Verify the impact of updated dependencies. # Test: Run go mod tidy to ensure all dependencies are properly aligned go mod tidy # Test: Check if there are any indirect dependencies that can be made direct go mod why -m all # Test: Verify that all dependencies are being used go mod why -m $(go list -m all)Additionally, review the changelog or release notes for each updated dependency to understand the changes and potential impacts on your codebase.
3-3: Go version update approved.The update to Go 1.23 aligns with the PR objective. This brings new features and improvements to the project.
Please ensure that all parts of the codebase are compatible with Go 1.23. Run the following script to check for any compatibility issues:
Line range hint
79-87: Dependency replacements approved.The updates to the
replacedirectives address security vulnerabilities and broken dependencies:
- Replacing
github.com/dgrijalva/jwt-gowithgh.neting.cc/golang-jwt/jwt/v4addresses the deprecation of the former and potential security issues.- Updating
github.com/gin-gonic/ginto v1.9.0 fixes upstream vulnerabilities.- Replacing
github.com/syndtr/goleveldbwith a specific commit hash ensures a stable version.These changes align with best practices for maintaining secure and up-to-date dependencies.
Please verify that these replacements don't introduce any breaking changes to the project. Run the following script to check for any potential issues:
✅ Verification successful
Dependency Replacement Verification Scripts
The previous verification attempt failed due to the absence of the
gocommand. Please run the following scripts to check for any remaining references to the old dependencies:
Dependency Replacements Successfully Verified
No direct references to
github.com/dgrijalva/jwt-goorgh.neting.cc/syndtr/goleveldbexist in the application's source code. The replace directives ingo.modare correctly applied, and remaining references ingo.modandgo.sumare expected for dependency management.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of replaced dependencies. # Test: Check for any references to the old jwt-go package rg "github.com/dgrijalva/jwt-go" # Test: Verify Gin usage compatibility rg "github.com/gin-gonic/gin" # Test: Check for any direct usage of goleveldb rg "github.com/syndtr/goleveldb"Length of output: 349
Script:
#!/bin/bash # Description: Verify that old dependencies are no longer referenced in the codebase. # Test: Check for any references to the deprecated jwt-go package rg "github.com/dgrijalva/jwt-go" # Test: Check for any direct usage of goleveldb rg "github.com/syndtr/goleveldb"Length of output: 1078
Makefile (2)
144-144: LGTM: gofumpt version update is appropriate.The update of gofumpt from v0.4.0 to v0.7.0 is in line with the PR objective. This change will ensure that the project uses the latest version of the code formatter.
To ensure this update doesn't cause unexpected formatting changes, please run the following verification:
#!/bin/bash # Description: Verify the impact of gofumpt update on code formatting # Test: Compare formatting changes between old and new gofumpt versions OLD_VERSION="v0.4.0" NEW_VERSION="v0.7.0" # Function to format code with a specific gofumpt version format_with_version() { VERSION=$1 docker run --rm \ -v $(pwd):/app \ -w /app \ golang:1.23-alpine3.20 \ sh -c "go install mvdan.cc/gofumpt@$VERSION && find . -name '*.go' | xargs gofumpt -l" } # Compare formatting differences diff <(format_with_version $OLD_VERSION) <(format_with_version $NEW_VERSION) # If there's no output, there are no differences in formattingIf the script produces output, review the affected files to ensure the formatting changes are acceptable.
12-12: LGTM: Go version update looks good.The update from
golang:1.21-alpine3.17togolang:1.23-alpine3.20aligns with the PR objective of upgrading to Go 1.23. This change also includes an update to the Alpine base image, which is good for keeping dependencies current.To ensure this change doesn't introduce any unexpected issues, please verify the build process using this updated image:
This PR updates the project to use Go 1.23 and also upgrades gofumpt to version v0.7.0.
Additionally, the golangci-lint configuration has been adjusted by removing exportloopref (no longer needed since Go 1.22) and replacing it with copyloopvar.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores