build(deps): go 1.23, mockery 2.46.2, golangci-lint 1.61 #954
Conversation
Bumps [github.com/vektra/mockery/v2](https://github.com/vektra/mockery) from 2.41.0 to 2.46.2. - [Release notes](https://github.com/vektra/mockery/releases) - [Changelog](https://github.com/vektra/mockery/blob/master/docs/changelog.md) - [Commits](vektra/mockery@v2.41.0...v2.46.2) --- updated-dependencies: - dependency-name: github.com/vektra/mockery/v2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
WalkthroughThe changes in this pull request primarily involve updating the Go version from 1.22 to 1.23 across various components of the project, including GitHub Actions workflows, Dockerfiles, the README, and tutorial documentation. Additionally, the 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
CodeRabbit Configuration File (
|
![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 (16)
.github/workflows/lint.yml (2)
35-35: LGTM: Go version updated as intended.The Go version has been correctly updated to "^1.23", which aligns with the PR objective of upgrading to Go 1.23. This change will allow the use of Go 1.23 or any compatible higher version.
Consider if a more specific version constraint (e.g., "1.23.x") would be beneficial for reproducibility across different environments. This is optional and depends on your project's versioning strategy.
Update golangci-lint to the latest version (v1.61.0).
- The workflow currently uses golangci-lint v1.55, while the latest version available is v1.61.0. Updating will ensure you benefit from the latest features and bug fixes.
- The timeout for golangci-lint is set to 10 minutes. Consider monitoring linting performance to determine if this timeout remains appropriate.
🔗 Analysis chain
Line range hint
1-58: Consider updating golangci-lint versions and reviewing timeout.The workflow is well-structured and optimized. However, consider the following suggestions:
Check if there are newer versions available for:
- golangci-lint-action (currently v6.1.1)
- golangci-lint itself (currently v1.55)
The timeout for golangci-lint is set to 10 minutes. While this is generally sufficient, for larger codebases or more complex linting configurations, you might want to consider increasing this value.
To check for the latest versions, you can run:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check latest golangci-lint-action version echo "Latest golangci-lint-action version:" gh release list --repo golangci/golangci-lint-action --limit 1 # Check latest golangci-lint version echo "Latest golangci-lint version:" gh release list --repo golangci/golangci-lint --limit 1Length of output: 360
test/e2e/docker/Dockerfile (1)
Line range hint
1-78: Consider optimizing the build processWhile the current Dockerfile structure is good, there are a few optimizations that could potentially improve build time and efficiency:
- Use BuildKit's cache mounts for Go module cache to speed up subsequent builds.
- Consider using Go's build cache to speed up rebuilds.
- Evaluate if all installed dependencies are necessary for the e2e tests.
Here's an example of how you could implement these optimizations:
FROM golang:${GOLANG_VERSION}-alpine${ALIPNE_VERSION} AS base +# Enable BuildKit's cache mount for Go modules +RUN --mount=type=cache,target=/go/pkg/mod \ + --mount=type=cache,target=/root/.cache/go-build \ apk update && \ apk upgrade && \ apk --no-cache add bash git gmp-dev sudo cmake build-base libpcap-dev leveldb-dev && \ rm -rf /var/cache/apk/* WORKDIR /src/tenderdash # Fetch dependencies separately (for layer caching) COPY go.mod go.sum ./ -RUN go mod download +RUN --mount=type=cache,target=/go/pkg/mod \ + --mount=type=cache,target=/root/.cache/go-build \ + go mod download # ... (rest of the Dockerfile) RUN rm -r /src/tenderdash/third_party && ln -s /src/bls/third_party /src/tenderdash/third_party && \ cd test/e2e && make node && cp build/node /usr/bin/app && \ - go clean && go clean -cache -fuzzcache -testcache -modcache + go cleanThese changes utilize BuildKit's cache mounts to persist the Go module cache and build cache between builds, potentially speeding up subsequent builds. The final
go cleancommand is also modified to retain the caches for future use.Additionally, it might be worth reviewing the list of installed packages in the base image to ensure all are necessary for the e2e tests, potentially reducing the image size.
.github/actions/bls/action.yml (1)
Line range hint
63-63: Update approved, consider consistencyThe update of actions/cache from v2 to v2.1.2 is a good practice, as it likely includes bug fixes and improvements.
For consistency, consider updating the other instance of actions/cache on line 29 to also use v2.1.2:
- - uses: actions/cache@v2 + - uses: actions/[email protected].github/workflows/build.yml (1)
26-26: Summary: Go version successfully updated to 1.23 across all jobsThe Go version has been consistently updated to 1.23 across all jobs in the workflow (build, test_abci_cli, and test_apps). This change aligns with the PR objective of upgrading to Go 1.23.
Given the significance of this upgrade and the mention of breaking changes in Go 1.23, consider the following recommendations:
- Update the project's documentation to reflect the new Go version requirement.
- Review and update any Go version-specific code throughout the project.
- Ensure all team members and contributors are aware of the Go version change and its implications.
- Consider adding a Go version check in the CI/CD pipeline to prevent accidental use of incompatible versions.
- Update any development environment setup instructions to specify Go 1.23.
These steps will help ensure a smooth transition to Go 1.23 and maintain project stability.
Also applies to: 57-57, 82-82
DOCKER/Dockerfile (1)
Line range hint
1-138: Summary of Dockerfile changes and implicationsThe main changes to the Dockerfile are:
- Updating the Go version to 1.23
- Adding an ARG for the Alpine version (with a typo to be fixed)
These changes align with the PR objectives and should enable building the project with Go 1.23. However, there are some important considerations:
Dependency Compatibility: Ensure all project dependencies are compatible with Go 1.23. Review the
go.modfile and update dependencies if necessary.Build Process: Verify that the build process succeeds with the new Go version across all environments (local development, CI/CD pipelines).
Testing: Run the full test suite to ensure compatibility with Go 1.23. Update tests if needed to accommodate any breaking changes.
Documentation: Update any documentation that references the Go version or build process.
CI/CD: Ensure all CI/CD pipelines are updated to use Go 1.23.
Performance: Monitor the application's performance after the upgrade, as new Go versions can sometimes impact performance.
These steps will help ensure a smooth transition to Go 1.23 and minimize potential issues post-merge.
internal/state/mocks/store.go (1)
Line range hint
1-13: LGTM: File header and package declaration are correct.The file header correctly indicates that this is generated code, and the package declaration is appropriate for a mock implementation. All necessary imports are included.
Consider removing the blank line (line 3) after the initial comment for consistency, although this doesn't affect functionality.
docs/tutorials/go.md (5)
Line range hint
1-57: Update Go version and LGTM for project setupThe tutorial's introduction and setup instructions have been significantly improved, making it more accessible for beginners. However, the Go version mentioned (1.19.x) is not the latest. Consider updating it to the current stable version.
The new section on creating a Go project is clear and helpful. It provides a good starting point for users to begin their Tendermint Core application development.
Consider updating the Go version to the latest stable release (currently 1.20.x as of July 2023) to ensure users are working with the most up-to-date tools.
Line range hint
58-457: Excellent additions to application implementation, minor suggestion on error handlingThe new content provides a comprehensive explanation of ABCI and its role in Tendermint Core applications. The detailed code snippets for implementing
KVStoreApplicationare clear, well-structured, and adequately commented. The introduction of Badger as the key-value store is a good choice for providing persistence.In the
isValidfunction, consider wrapping thepanic(err)with a more graceful error handling mechanism. For example:if err != nil { // Log the error log.Printf("Error in isValid: %v", err) // Return an error code instead of panicking return 3 // or another appropriate error code }This approach would make the application more robust and easier to debug in production environments.
Line range hint
458-559: Clear setup instructions, but Go version inconsistencyThe additions provide excellent step-by-step instructions for setting up and running the application. The explanation of the
main.gofile is thorough and helpful for understanding the application's entry point.There's an inconsistency in the Go versions mentioned:
- Earlier in the tutorial, Go version 1.19.x was mentioned.
- In the
go.modfile snippet, Go version 1.23 is specified.Please update these to consistently reflect the recommended Go version for this tutorial. As of July 2023, the latest stable Go version is 1.20.x. Consider updating all references to this version.
Line range hint
560-624: Clear instructions for running and testing, suggestion for additional exampleThe instructions for building, running, and testing the application are clear and comprehensive. The curl commands provided demonstrate the basic functionality of the key-value store effectively.
Consider adding an example of how to query a non-existent key to demonstrate the application's behavior in that scenario. This could help users better understand the full range of responses from the application. For example:
$ curl -s 'localhost:26657/abci_query?data="nonexistent_key"'This addition would provide a more complete picture of the application's functionality.
Line range hint
625-631: Good conclusion, opportunity for expansionThe outro provides a nice wrap-up for the tutorial and appropriately directs users to further documentation.
Consider expanding this section to provide more guidance on potential next steps. For example:
- Suggest some advanced topics that users could explore next (e.g., consensus mechanisms, network configuration, etc.).
- Mention any community resources where users can get help or discuss Tendermint Core development.
- Provide a link to any sample projects or real-world applications built with Tendermint Core that users could study.
These additions would give users a clearer path for continuing their learning journey with Tendermint Core.
docs/tutorials/go-built-in.md (4)
599-599: Approve changes toKVStoreApplicationstructure and methodsThe expanded explanation of the
KVStoreApplicationtype and its methods, including the addition ofNewKVStoreApplicationand the updatedCheckTxmethod with validation logic, greatly improves the tutorial's clarity. These changes provide a more comprehensive understanding of the application structure for beginners.Consider adding a brief explanation of why validation is performed again in
DeliverTx, as this might not be immediately obvious to beginners.
599-599: Approve Badger database integration detailsThe addition of detailed explanations on initializing the Badger database and managing transactions, including the updated
Commitmethod with database commit logic, significantly enhances the tutorial. These changes provide a clearer understanding of data persistence in the application.Consider adding a brief section on error handling best practices when working with the database, particularly for production environments.
599-599: Approve enhanced main function and application setupThe expanded main function with more robust setup, configuration, and the addition of signal handling for graceful shutdown significantly improves the application's production-readiness. These changes are crucial for maintaining data integrity and providing a more stable application.
Consider adding a brief comment explaining the purpose of the signal handling code, as this might not be immediately clear to beginners.
599-599: Approve updated build and run instructionsThe refined instructions for building and running the application, including the new steps for initializing Tendermint, greatly improve the tutorial's usability. These clear and comprehensive instructions are particularly beneficial for beginners new to Tendermint.
Consider adding a troubleshooting section or linking to common issues and their solutions, as this could be helpful for users who encounter problems during setup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (15)
- .github/actions/bls/action.yml (1 hunks)
- .github/workflows/build.yml (3 hunks)
- .github/workflows/check-generated.yml (2 hunks)
- .github/workflows/e2e.yml (1 hunks)
- .github/workflows/lint.yml (1 hunks)
- .github/workflows/release.yml (1 hunks)
- .github/workflows/tests.yml (1 hunks)
- DOCKER/Dockerfile (1 hunks)
- README.md (1 hunks)
- docs/tutorials/go-built-in.md (2 hunks)
- docs/tutorials/go.md (1 hunks)
- go.mod (1 hunks)
- internal/state/mocks/store.go (1 hunks)
- test/e2e/docker/Dockerfile (1 hunks)
- test/fuzz/README.md (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/fuzz/README.md
🧰 Additional context used
🔇 Additional comments (36)
.github/workflows/release.yml (2)
20-20: Please clarify the purpose of modifying the Build step condition.The
ifcondition for theBuildstep has been changed to run only on pull request events. This modification doesn't seem directly related to the Go version upgrade mentioned in the PR objectives.Could you please explain:
- The reasoning behind this change?
- How this affects the release process?
- Whether this change should be part of this PR or if it would be better suited for a separate PR focused on workflow improvements?
To understand the impact, let's check for other uses of this build step:
✅ Verification successful
The modification to the Build step condition is appropriate and isolated to the release workflow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other uses of the goreleaser build step echo "Checking for other goreleaser build steps in workflow files:" rg --type yaml "goreleaser/goreleaser-action@v6" .github/workflows -A 5 -B 2Length of output: 2100
17-17: LGTM! Verify version consistency across the project.The Go version has been correctly updated to 1.23, which aligns with the PR objective.
To ensure consistency, let's verify the Go version across other relevant files:
✅ Verification successful
Go version has been consistently updated to 1.23 across the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for Go version consistency across the project # Test 1: Check Go version in Dockerfiles echo "Checking Dockerfiles for Go version:" rg --type dockerfile "FROM golang:" -A 1 # Test 2: Check Go version in go.mod echo "Checking go.mod for Go version:" rg --type go "go \d+\.\d+" go.mod # Test 3: Check Go version in other workflow files echo "Checking other workflow files for Go version:" rg --type yaml "go-version:" .github/workflowsLength of output: 2100
.github/workflows/tests.yml (2)
Line range hint
1-47: Verify workflow compatibility with Go 1.23While the Go version update is correct, please review the following aspects of the workflow to ensure full compatibility with Go 1.23:
- Verify that the matrix strategy and test parts (00-05) are still appropriate for the new Go version.
- Review the dependency installation steps (e.g., libpcap, BLS library) to confirm they are compatible with Go 1.23.
- Check if the environment variables (CGO_LDFLAGS, CGO_CXXFLAGS) need any adjustments for the new Go version.
- Consider adding a step to explicitly verify the Go version being used during the workflow execution.
To help verify the compatibility, you can run the following script to check for any Go-related changes in the project:
#!/bin/bash # Description: Check for recent Go-related changes in the project # Check for recent changes in Go files echo "Recent changes in Go files:" git log -n 5 --pretty=format:"%h - %s" --grep="go" -- '*.go' # Check for recent changes in go.mod and go.sum echo -e "\nRecent changes in go.mod and go.sum:" git log -n 5 --pretty=format:"%h - %s" -- go.mod go.sum # List all Go-related files changed in this PR echo -e "\nGo-related files changed in this PR:" git diff --name-only origin/master...HEAD | grep -E '\.go$|go\.mod|go\.sum'This script will help identify recent Go-related changes that might require adjustments in the workflow.
21-21: Go version update looks good, but consider broader implications.The update of the Go version to 1.23 is correct and aligns with the PR objectives. However, please consider the following:
- Ensure that this change is consistent across all relevant files in the project (e.g., Dockerfiles, README, etc.).
- Verify if any other parts of this workflow or other workflows need adjustments due to potential breaking changes in Go 1.23.
- Consider adding a step to verify the Go version during the workflow execution to catch any potential discrepancies.
To help verify the consistency of the Go version across the project, you can run the following script:
This script will help identify any inconsistencies in Go version references across the project.
test/e2e/docker/Dockerfile (1)
3-3: LGTM: Go version updated to 1.23The Go version has been successfully updated to 1.23 as per the PR objective. This change is crucial for addressing issue #944.
To ensure compatibility with Go 1.23 throughout the build process, please run the following verification script:
This script will help identify any inconsistencies or potential compatibility issues related to the Go version upgrade.
.github/workflows/check-generated.yml (4)
26-26: LGTM: Go version update forcheck-mocksjobThe Go version has been correctly updated to 1.23, which aligns with the PR objective to upgrade the Go version.
52-52: LGTM: Go version update forcheck-protojobThe Go version has been correctly updated to 1.23 for the
check-protojob, maintaining consistency with thecheck-mocksjob and aligning with the PR objective.
Line range hint
55-57: Excellent addition: Ensuring Git history for diff checkThe new
withclause specifyingfetch-depth: 1is a valuable addition. It ensures that a.gitdirectory is available for runninggit diff, which is crucial for the protobuf generated code check. The comment clearly explains the purpose of this change, enhancing the workflow's maintainability.
26-26: Summary: Successful Go version upgrade and workflow improvementThe changes in this file successfully upgrade the Go version to 1.23 for both the
check-mocksandcheck-protojobs, aligning with the PR objective. Additionally, the workflow has been improved by ensuring the availability of Git history for the diff check in thecheck-protojob.These changes enhance the workflow's functionality and maintain consistency with the project's upgrade goals. Great job on implementing these updates!
Also applies to: 52-57
.github/actions/bls/action.yml (2)
18-18: Go version update approved, verify compatibilityThe Go version update from 1.22 to 1.23 aligns with the PR objective. This change ensures that the BLS library is built using the latest Go version.
To ensure compatibility across the project, please run the following script to check for any other Go version references that might need updating:
#!/bin/bash # Description: Check for any remaining references to Go 1.22 in the project # Search for Go 1.22 references in all files echo "Searching for Go 1.22 references:" rg --type yaml --type dockerfile --type md "go-version:?\s*\"?1\.22\"?" -g '!.github/actions/bls/action.yml' # Search for go.mod files and check their Go version echo -e "\nChecking go.mod files for Go version:" fd -e mod -x sh -c "echo {} && sed -n 's/^go //p' {}"
Line range hint
1-70: Ensure comprehensive testing and documentation updatesWhile the Go version update has been implemented in this file, there are some important follow-up actions to consider:
Comprehensive Testing: Given that Go 1.23 introduces breaking changes, it's crucial to update and run all relevant unit, integration, functional, and end-to-end tests to ensure compatibility.
Documentation Updates: The project documentation should be reviewed and updated to reflect the new Go version requirement and any changes in behavior or APIs that may have resulted from this upgrade.
Dependency Review: Conduct a thorough review of all project dependencies to ensure they are compatible with Go 1.23.
To assist with these tasks, consider running the following script to identify test files and documentation that may need updating:
✅ Verification successful
Go version update verification successful
All documentation, test files, and configuration files have been successfully updated to Go 1.23. No lingering references to Go 1.22 were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Identify test files and documentation that may need updating echo "Go test files that may need updating:" fd -e go -e mod --type f --exec grep -l 'testing' {} echo -e "\nDocumentation files that may need reviewing:" fd -e md -e txt -e rst --type f --exec grep -il 'go\s*1\.22' {}Length of output: 387
Script:
#!/bin/bash # Description: Identify documentation files that may need updating from Go 1.22 to 1.23 echo "Searching for Go version references in documentation files (excluding dockerfiles):" rg --type md --type txt --type rst 'go\s*1\.22' --ignore-file .gitignore echo -e "\nChecking go.mod files for Go version consistency:" fd -e mod -x sh -c 'echo {} && sed -n '\''s/^go //p'\'' {}'Length of output: 442
Script:
#!/bin/bash # Description: Verify that all test files are updated to use Go 1.23 echo "Searching for Go 1.22 references in test files:" rg --type go --type go.mod --type go.sum 'go\s*1\.22' --glob '!*.test.go' --ignore-file .gitignore echo -e "\nChecking test-specific configuration files for Go version consistency:" fd -e yml -e yaml -e json -e toml --exec grep -H 'go\s*1\.22' {}Length of output: 462
Script:
#!/bin/bash # Description: Verify that all test files and test configurations are updated to use Go 1.23 echo "Searching for Go 1.22 references in test files (*.test.go):" rg 'go\s*1\.22' --glob '*.test.go' --ignore-file .gitignore echo -e "\nSearching for Go 1.22 references in test configuration files (yml, yaml, json, toml):" rg 'go\s*1\.22' --type yml --type yaml --type json --type toml --ignore-file .gitignoreLength of output: 644
Script:
#!/bin/bash # Description: Verify that all test files and test configurations are updated to use Go 1.23 echo "Searching for Go 1.22 references in test files (*.test.go):" rg 'go\s*1\.22' --glob '*_test.go' --ignore-file .gitignore echo -e "\nSearching for Go 1.22 references in test configuration files (yaml, json, toml):" rg 'go\s*1\.22' --type yaml --type json --type toml --ignore-file .gitignoreLength of output: 446
.github/workflows/build.yml (3)
57-57: Approved: Go version update to 1.23 in test_abci_cli jobThe Go version has been correctly updated to 1.23 in the 'test_abci_cli' job, maintaining consistency with the 'build' job.
To ensure the smooth execution of the ABCI CLI tests with Go 1.23:
- Verify that all ABCI CLI tests are compatible with Go 1.23.
- Run the ABCI CLI tests locally with Go 1.23 to catch any potential issues before merging.
Run the following script to check for any Go version-specific code in the ABCI CLI tests:
#!/bin/bash # Description: Check for Go version-specific code in ABCI CLI tests # Test: Search for version-specific imports or syntax in ABCI CLI test files echo "Checking ABCI CLI test files for version-specific code:" rg -i -t go 'go1\.[0-9]+' abci/tests/
82-82: Approved: Go version update to 1.23 in test_apps jobThe Go version has been correctly updated to 1.23 in the 'test_apps' job, maintaining consistency across all jobs in the workflow.
To ensure the smooth execution of the app tests with Go 1.23:
- Verify that all app tests are compatible with Go 1.23.
- Run the app tests locally with Go 1.23 to catch any potential issues before merging.
Run the following script to check for any Go version-specific code in the app tests:
#!/bin/bash # Description: Check for Go version-specific code in app tests # Test: Search for version-specific imports or syntax in app test files echo "Checking app test files for version-specific code:" rg -i -t go 'go1\.[0-9]+' test/app/
26-26: Approved: Go version update to 1.23The Go version has been correctly updated to 1.23 in the 'build' job, which aligns with the PR objective.
Given that Go 1.23 introduces breaking changes, please ensure that:
- All dependencies are compatible with Go 1.23.
- The project's code has been reviewed for any necessary adjustments due to breaking changes.
- CI/CD pipelines and deployment processes are updated to use Go 1.23.
Run the following script to verify Go version consistency across the repository:
✅ Verification successful
Go version successfully updated to 1.23
All Go version specifications across YAML files and
go.modhave been correctly updated to "1.23". No remaining references to Go 1.22 were found, and there are no Dockerfiles present that require updates.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for Go version consistency across the repository # Test: Search for Go version specifications echo "Checking Go version specifications:" rg --type yaml -i 'go-version|golang' .github/ rg --type dockerfile -i 'go-version|golang' rg --type go -i 'go 1\.[0-9]+' go.mod # Test: Check if there are any remaining references to Go 1.22 echo "Checking for remaining references to Go 1.22:" rg -i 'go 1\.22' .Length of output: 433
Script:
#!/bin/bash # Description: Check for Go version consistency across the repository # Test: Search for Go version specifications in .github workflows and Dockerfiles echo "Checking Go version specifications in YAML and Dockerfiles:" rg --type yaml -i 'go-version|golang' .github/ rg --type dockerfile -i 'go-version|golang' . # Test: Search for Go version specifications in go.mod files echo "Checking Go version specifications in go.mod files:" rg --type go -i 'go 1\.[0-9]+' go.mod # Test: Check if there are any remaining references to Go 1.22 echo "Checking for remaining references to Go 1.22:" rg -i 'go 1\.22' .Length of output: 1736
Script:
#!/bin/bash # Description: Enhanced check for Go version consistency across the repository # Test: Search for Go version specifications in YAML and Dockerfiles without using --type echo "Checking Go version specifications in YAML files:" rg --type yaml -i 'go-version|golang' .github/ echo "Checking Go version specifications in Dockerfiles:" rg -i 'go-version|golang' '**/Dockerfile*' # Test: Search for Go version specifications in go.mod files echo "Checking Go version specifications in go.mod files:" rg --type go -i 'go 1\.[0-9]+' go.mod # Test: Check if there are any remaining references to Go 1.22 echo "Checking for remaining references to Go 1.22:" rg -i 'go 1\.22' .Length of output: 1844
.github/workflows/e2e.yml (2)
Line range hint
1-92: Verify compatibility of build process with Go 1.23While the workflow structure remains unchanged, it's important to ensure that all build processes are compatible with Go 1.23. Please verify the following:
- Check if the Dockerfile used in the Docker build step needs to be updated to use Go 1.23.
- Verify that the make commands for building runner and tests are compatible with Go 1.23.
Let's check the Dockerfile and make commands:
#!/bin/bash # Description: Check Dockerfile and make commands for potential updates # Check Dockerfile echo "Checking Dockerfile for Go version:" cat test/e2e/docker/Dockerfile | grep -i 'FROM golang:' # Check make commands echo -e "\nChecking make commands in test/e2e/Makefile:" cat test/e2e/Makefile | grep -E 'go |build'Please review the output and ensure that:
- The Dockerfile is using the correct Go version (1.23).
- The make commands don't have any version-specific flags or commands that might be incompatible with Go 1.23.
If any changes are needed, please update the respective files accordingly.
31-31: Go version update looks good, but let's verify consistency.The update to Go 1.23 is correctly implemented and aligns with the PR objective. However, we should ensure this change is consistent across all relevant files in the project.
Let's verify the Go version consistency:
Please review the output to ensure all references to the Go version have been updated to 1.23.
✅ Verification successful
Please execute the above script to verify that all Go version references are consistently updated to 1.23 across the project.
Please execute the above script to verify that all Go version references are consistently updated to 1.23 across the project.
Go version update is consistent across the project.
All references to the Go version have been successfully updated to 1.23 in the relevant files. The README badge dynamically reflects the updated Go version based on
go.mod.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for Go version consistency across the project # Search for Go version specifications echo "Searching for Go version specifications:" rg --type yaml --type dockerfile --type md -i 'go-version|golang:|FROM golang:' -g '!vendor/**' # Search for go.mod file echo -e "\nChecking go.mod file:" cat go.mod | grep -i 'go 1.'Length of output: 1110
Script:
#!/bin/bash # Description: Check for Go version consistency across the project # Search for Go version specifications in YAML, Dockerfile, and Markdown files echo "Searching for Go version specifications in YAML, Dockerfile, and Markdown files:" rg --type yaml --type dockerfile --type md -i 'go-version|golang:|FROM golang:' -g '!vendor/**' # Check the Go version specified in go.mod echo -e "\nChecking Go version in go.mod:" grep -E '^go\s+1\.23' go.modLength of output: 419
Script:
#!/bin/bash # Description: Check for Go version consistency across the project # Search for Go version specifications in YAML, Dockerfile, and Markdown files echo "Searching for Go version specifications in YAML and Markdown files:" rg --type yaml --type markdown -i 'go-version|golang:' -g '!vendor/**' echo -e "\nSearching for Go version in Dockerfiles:" # Specifically search for files named Dockerfile (case-insensitive) fd -i 'Dockerfile' | xargs grep -Ei 'FROM golang:1\.23' # Check the Go version specified in go.mod echo -e "\nChecking Go version in go.mod:" grep -E '^go\s+1\.23' go.modLength of output: 626
DOCKER/Dockerfile (1)
8-8: Approve Go version update and suggest compatibility checksThe Go version update to 1.23 is correctly implemented, aligning with the PR objective.
Given that this update introduces breaking changes, please ensure the following:
- All project dependencies are compatible with Go 1.23.
- The build process succeeds with the new Go version.
- All tests pass with Go 1.23.
Run the following script to verify Go version and build process:
If any issues are encountered, they should be addressed before merging this PR.
README.md (2)
Line range hint
1-50: README.md is up-to-date and consistent.The README.md file has been successfully updated with the new Go version requirement. The rest of the file remains unchanged and consistent with the project's description and guidelines.
Also applies to: 51-100
50-50: Approved: Go version requirement updated correctly.The minimum Go version has been correctly updated from 1.22 to 1.23, which aligns with the PR objectives. However, this is a breaking change that requires users to upgrade their Go version.
Please ensure that all other references to the Go version in the codebase have been updated accordingly. Run the following script to check for any remaining mentions of Go 1.22:
Consider adding a note about this breaking change to the CHANGELOG.md file to ensure users are aware of the new Go version requirement.
✅ Verification successful
Please run the following script to check for any remaining mentions of Go 1.22, excluding
go.modandgo.sumfiles:
Verified: No remaining references to Go 1.22 found.
All references to the Go version have been successfully updated to 1.23. The mentions in
docs/package-lock.jsonpertain to package versions and do not affect the Go version requirement.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining mentions of Go 1.22 in the codebase # Search for "1.22" or "go1.22" in all files rg -i '(1\.22|go1\.22)' --type-not go.mod --type-not go.sumLength of output: 92
Script:
# #!/bin/bash # Description: Check for any remaining mentions of Go 1.22 in the codebase excluding go.mod and go.sum rg -i '(1\.22|go1\.22)' --glob '!go.mod' --glob '!go.sum'Length of output: 465
internal/state/mocks/store.go (12)
Line range hint
15-18: LGTM: Store struct declaration is correct.The
Storestruct is properly declared with the embeddedmock.Mock, which is the standard pattern for testify mocks.
Line range hint
20-36: LGTM: Bootstrap method implementation is correct.The
Bootstrapmethod is properly implemented following the testify mock pattern. It correctly handles different return value scenarios and includes appropriate error checking.
Line range hint
38-54: LGTM: Close method implementation is correct.The
Closemethod is properly implemented following the testify mock pattern. It correctly handles different return value scenarios and includes appropriate error checking.
Line range hint
56-83: LGTM: Load method implementation is correct.The
Loadmethod is properly implemented following the testify mock pattern. It correctly handles different return value scenarios for both return values (state.State and error) and includes appropriate error checking.
Line range hint
85-115: LGTM: LoadABCIResponses method implementation is correct.The
LoadABCIResponsesmethod is properly implemented following the testify mock pattern. It correctly handles different return value scenarios for both return values (*tendermintstate.ABCIResponses and error) and includes appropriate error checking.
Line range hint
117-143: LGTM: LoadConsensusParams method implementation is correct.The
LoadConsensusParamsmethod is properly implemented following the testify mock pattern. It correctly handles different return value scenarios for both return values (types.ConsensusParams and error) and includes appropriate error checking.
Line range hint
145-175: LGTM: LoadValidators method implementation is correct.The
LoadValidatorsmethod is properly implemented following the testify mock pattern. It correctly handles different return value scenarios for both return values (*types.ValidatorSet and error) and includes appropriate error checking.
Line range hint
177-193: LGTM: PruneStates method implementation is correct.The
PruneStatesmethod is properly implemented following the testify mock pattern. It correctly handles different return value scenarios and includes appropriate error checking.
Line range hint
195-211: LGTM: Save method implementation is correct.The
Savemethod is properly implemented following the testify mock pattern. It correctly handles different return value scenarios and includes appropriate error checking.
Line range hint
213-229: LGTM: SaveABCIResponses method implementation is correct.The
SaveABCIResponsesmethod is properly implemented following the testify mock pattern. It correctly handles different return value scenarios and includes appropriate error checking.
Line range hint
231-265: LGTM: SaveValidatorSets method and NewStore function implementations are correct.The
SaveValidatorSetsmethod is properly implemented following the testify mock pattern. It correctly handles different return value scenarios and includes appropriate error checking.The
NewStorefunction correctly creates a new instance of theStoremock, sets up the test interface, and registers a cleanup function to assert expectations after the test. This follows best practices for setting up testify mocks.
Line range hint
1-265: Overall: The generated Store mock is well-implemented and follows best practices.This generated mock for the
Storeinterface is comprehensive and adheres to best practices for testify mocks. All methods are correctly implemented, with proper error handling and return value management. TheNewStorefunction provides a convenient way to create and set up the mock for testing.This mock should serve as a reliable tool for testing code that depends on the
Storeinterface, allowing for easy setup of expected behaviors and verification of interactions.go.mod (4)
Line range hint
298-299: Verify compatibility with updated dependenciesThe following dependencies have been updated:
- golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
- golang.org/x/time v0.5.0
Please ensure that these updates don't introduce any breaking changes to the project. Run the following script to check for any potential issues:
#!/bin/bash # Description: Check for potential issues with updated dependencies # Test: Build the project and run tests go build ./... && go test ./... # Test: Check for any deprecation warnings or errors related to updated packages go vet ./... 2>&1 | grep -E "golang.org/x/exp|golang.org/x/time"
Line range hint
1-299: Conduct comprehensive testing after dependency changesGiven the significant changes in the
go.modfile, including the Go version update, new dependencies, and updated dependencies, it's crucial to conduct comprehensive testing of the project.Please ensure the following:
- The project builds successfully with the new Go version and updated dependencies.
- All unit tests pass.
- Integration tests (if any) pass.
- There are no performance regressions.
Run the following script to perform a basic check:
Please review the results of these tests and address any issues that may arise.
3-3: Verify compatibility with Go 1.23The Go version has been updated from 1.22 to 1.23. This major version update may introduce breaking changes.
Please ensure that:
- All project dependencies are compatible with Go 1.23.
- The project builds successfully with the new Go version.
- All tests pass with Go 1.23.
Run the following script to check for any compatibility issues:
Line range hint
52-57: Clarify the purpose of new dependenciesSeveral new dependencies have been added to the project:
- github.com/bufbuild/buf v1.35.1
- github.com/creachadair/atomicfile v0.2.6
- github.com/creachadair/taskgroup v0.3.2
- github.com/go-pkgz/jrpc v0.2.0
- github.com/google/go-cmp v0.6.0
- github.com/vektra/mockery/v2 v2.41.0
Could you please provide more information on why these dependencies were added and how they will be used in the project? This will help ensure that they are necessary and don't introduce any potential conflicts or security risks.
Run the following script to check for any potential security vulnerabilities in these new dependencies:
docs/tutorials/go-built-in.md (1)
46-46: Verify Go version consistency throughout the documentThe Go version requirement has been updated from 1.22.x to 1.23.x. Please ensure that this change is reflected consistently throughout the entire document and aligns with the project's overall requirements.
✅ Verification successful
Go version updated consistently across the repository
All references to Go 1.22 have been successfully updated to Go 1.23.x.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining references to Go 1.22 rg "go1\.22" docs/tutorials/go-built-in.mdLength of output: 44
Script:
#!/bin/bash # Search for any references to Go 1.22 in the entire repository rg "go1\.22"Length of output: 14
…/vektra/mockery/v2-2.46.2' into build/go-1.23
lklimek
changed the title
lklimek
changed the title
lklimek
force-pushed
the
build/go-1.23
branch
from
5e84ee7 to
809073e
Compare
|
Failing tests:
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
internal/p2p/mocks/transport.go (1)
9-9: LGTM. Consider establishing a consistent import order policy.The change in import order doesn't affect functionality. However, for better maintainability, consider establishing a consistent import order policy across the project (e.g., standard library first, then third-party, then local imports).
Makefile (1)
295-295: LGTM! Consider using a variable for the golangci-lint version.The update to use a specific version (v1.61.0) of golangci-lint is good. It ensures consistency across different environments and prevents potential issues from unexpected changes in newer versions.
Consider defining a variable for the golangci-lint version at the top of the Makefile, like this:
GOLANGCI_LINT_VERSION := v1.61.0Then use it in the lint command:
go run github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) runThis approach will make it easier to update the version in the future, as you'll only need to change it in one place.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
- .github/workflows/check-generated.yml (2 hunks)
- .github/workflows/e2e.yml (1 hunks)
- .github/workflows/lint.yml (2 hunks)
- DOCKER/Dockerfile (2 hunks)
- Makefile (1 hunks)
- go.mod (10 hunks)
- internal/p2p/client/mocks/block_client.go (1 hunks)
- internal/p2p/mocks/connection.go (2 hunks)
- internal/p2p/mocks/transport.go (2 hunks)
- libs/store/mocks/store.go (2 hunks)
- scripts/mockery_generate.sh (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- scripts/mockery_generate.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/check-generated.yml
- .github/workflows/e2e.yml
- .github/workflows/lint.yml
- DOCKER/Dockerfile
🧰 Additional context used
🔇 Additional comments (15)
internal/p2p/client/mocks/block_client.go (2)
Line range hint
71-83: LGTM: Appropriate use ofanytypeThe change from
interface{}toanyin theSendmethod signature and implementation is a good update. This aligns with modern Go practices introduced in Go 1.18, whereanyis a more readable alias forinterface{}. The change doesn't affect the functionality but improves code clarity.
Line range hint
1-95: Summary: Generated mock updated with Go 1.18+ syntaxThis file has been regenerated using an updated version of mockery, which now uses the
anytype alias introduced in Go 1.18. The change is isolated to theSendmethod, updating both its signature and implementation. This update:
- Aligns the mock with modern Go practices.
- Maintains backward compatibility, as
anyis just an alias forinterface{}.- Improves code readability without changing functionality.
No other changes were made to the file, and the overall structure remains consistent with mockery's output.
libs/store/mocks/store.go (3)
11-11: Approved: Modernized type constraint syntaxThe change from
interface{}toanyfor theVtype parameter is a positive modernization of the code.anyis an alias forinterface{}introduced in Go 1.18, making the code more idiomatic for recent Go versions. This change aligns well with the PR objective of upgrading to Go 1.23 and improves code readability without affecting behavior.
171-174: Approved: Consistent use of modernized type constraintThe change in the
NewStorefunction signature fromV interface{}toV anyis consistent with the earlier modification to theStorestruct. This ensures uniformity in the use of type constraints throughout the file and aligns with modern Go conventions introduced in Go 1.18+. The change maintains the function's behavior while improving code consistency and readability.
Line range hint
1-181: Verify compatibility with updated mockery versionThe changes in this file are syntactical updates aligning with modern Go practices, specifically the use of
anyinstead ofinterface{}. These changes are backwards compatible and don't introduce functional modifications, which reduces the risk of breaking existing code.Given that this is a generated mock file, it's important to ensure that these changes are consistent with the upgraded mockery version (2.46.2) mentioned in the PR objectives.
To verify this, please run the following command:
This will confirm the mockery version and regenerate the mocks, allowing us to verify that the changes are indeed coming from the updated mockery version and that no unexpected modifications were introduced.
internal/p2p/mocks/transport.go (1)
Line range hint
1-190: Overall, the changes look good but require verification.The modifications to this generated mock file are minimal and appear to be part of a larger refactoring effort. The main changes (import order and
ChannelDescriptortype) don't introduce any apparent issues. However, as this is a generated file, it's crucial to ensure that these changes are consistent with the actualTransportinterface implementation and other parts of the codebase that interact with it.Please ensure that:
- The actual
Transportinterface in thep2ppackage has been updated to match these changes.- Any code that implements or uses the
Transportinterface has been updated accordingly.- The mock generation process (likely using
mockery) is up-to-date and correctly configured to generate these changes.internal/p2p/mocks/connection.go (3)
Line range hint
1-200: Overall assessment: Changes look good and align with PR objectives.The modifications to the
Connectionmock in this file are consistent with the PR objectives of upgrading Go and dependencies. The changes fromconn.ChannelIDtop2p.ChannelIDin bothReceiveMessageandSendMessagemethods maintain consistency with thep2ppackage.To ensure a smooth transition, please run the verification scripts provided in the previous comments to identify any areas of the codebase that might need updates due to these changes. Additionally, make sure to update and run all relevant tests that use this mock to confirm there are no breaking changes.
152-163: LGTM! Verify mock usage across the codebase.The changes to the
SendMessagemethod signature and implementation look good. The update fromconn.ChannelIDtop2p.ChannelIDensures consistency with thep2ppackage.To ensure that these changes don't break existing tests or implementations, please run the following script to verify the usage of this mock across the codebase:
This script will help identify any potential areas that might need updates due to the signature change.
✅ Verification successful
Verification Complete. No issues found.
The
SendMessagemethod changes inmock.Connectiondo not appear to impact other parts of the codebase based on the current analysis.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of Connection mock and SendMessage method # Test: Search for Connection mock usage echo "Searching for Connection mock usage:" rg --type go "mock\.Connection" -C 3 # Test: Search for SendMessage calls on Connection mock echo "Searching for SendMessage calls on Connection mock:" rg --type go "\.SendMessage\(" -C 3Length of output: 3254
97-113: LGTM! Verify mock usage across the codebase.The changes to the
ReceiveMessagemethod signature and implementation look good. The update fromconn.ChannelIDtop2p.ChannelIDensures consistency with thep2ppackage.To ensure that these changes don't break existing tests or implementations, please run the following script to verify the usage of this mock across the codebase:
This script will help identify any potential areas that might need updates due to the signature change.
go.mod (6)
21-21: Review golangci-lint configuration with v1.61.0The
golangci-linttool has been updated tov1.61.0. This version may include new linters, deprecations, or changes in existing linters that could affect linting results. Please review your.golangci.ymlconfiguration to ensure compatibility and adjust any settings as necessary.
43-43: Update to golang.org/x/net v0.28.0The dependency
golang.org/x/nethas been updated tov0.28.0. Ensure that this update doesn't introduce any breaking changes, especially if there were any deprecated or altered APIs between versions.
45-45: Verify compatibility with gRPC v1.66.0The
google.golang.org/grpcpackage has been updated fromv1.64.1tov1.66.0. Please verify that the new version is compatible with your existing gRPC implementations and that there are no breaking changes affecting your codebase.
55-55: Update to mockery v2.46.2The
github.com/vektra/mockery/v2dependency has been updated tov2.46.2as required for compatibility with Go 1.23. Ensure that all generated mocks have been regenerated using this version and that your tests are passing.
66-75: Review newly added indirect dependenciesSeveral new indirect dependencies have been added:
github.com/4meepo/tagalign v1.3.4github.com/Abirdcfly/dupword v0.1.1github.com/Antonboom/testifylint v1.4.3github.com/Crocmagnon/fatcontext v0.5.2github.com/GaijinEntertainment/go-exhaustruct/v3 v3.3.0github.com/Masterminds/semver/v3 v3.3.0github.com/OpenPeeDeeP/depguard/v2 v2.2.0github.com/alecthomas/go-check-sumtype v0.1.4github.com/alexkohler/nakedret/v2 v2.0.4github.com/antlr4-go/antlr/v4 v4.13.1It's a good practice to review these indirect dependencies for any potential licensing, security, or compatibility concerns that might affect your project.
3-3: Update to Go 1.23The
go.modfile now specifiesgo 1.23, upgrading from the previous version. This aligns with the PR objective to support the new version of mockery (v2.46.2). Please ensure that all team members and CI/CD pipelines are updated to use Go 1.23 to prevent any build or runtime issues.You can run the following script to verify that all
go.modfiles specify the correct Go version:✅ Verification successful
Verification Successful: All
go.modfiles specifygo 1.23.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all go.mod files specify 'go 1.23'. # Expected output: All go.mod files should show 'go 1.23'. fd go.mod -x grep -H '^go ' {}Length of output: 51
Issue being fixed or feature implemented
Go 1.23 is required by mockery 2.46.2 #944
What was done?
Upgraded Go to 1.23
Upgraded mockery to 2.46.2 and regenerated mocks
Upgraded golanci-lint to 1.61
How Has This Been Tested?
GHA
Breaking Changes
Go 1.23 is required to build
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Documentation
Bug Fixes
Chores