Skip to content

Comments

chore: bump ev-node imports and docker upgrade test#2818

Merged
tac0turtle merged 2 commits intomainfrom
marko/single_docker-tests
Nov 4, 2025
Merged

chore: bump ev-node imports and docker upgrade test#2818
tac0turtle merged 2 commits intomainfrom
marko/single_docker-tests

Conversation

@tac0turtle
Copy link
Contributor

Overview

this pr bumps ev-node in go.mods and in our docker upgrade test

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2025

The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped✅ passed✅ passedNov 4, 2025, 3:04 PM

@claude
Copy link
Contributor

claude bot commented Nov 4, 2025

Claude finished @tac0turtle's task —— View job


Code Review Complete ✓

Summary

This PR performs a straightforward dependency version bump from v1.0.0-beta.8 to v1.0.0-beta.9 across multiple Go modules and updates the Docker upgrade test to use the new version as the baseline. The changes are minimal, focused, and appropriate for a chore-type PR.

Review Findings

Positive Aspects

  1. Consistency: The version bump is applied consistently across all 5 go.mod files that reference github.com/evstack/ev-node
  2. Test Alignment: The Docker upgrade test baseline version is updated to match the new dependency version, maintaining test accuracy
  3. Clean Changes: Only 10 lines changed across 8 files - exactly what's needed, nothing more
  4. go.sum Updates: The checksum files are properly updated with corresponding hash changes

📋 Code Quality & Architecture

Status: ✅ Approved

  • The modular go.mod structure follows Go best practices with separate modules for different components
  • The replace directives are used appropriately for local development
  • No cyclic dependencies or architectural concerns introduced

🔒 Security

Status: ✅ No Issues Found

  • Version bump from beta.8 → beta.9 is a minor increment within the same beta series
  • No new dependencies introduced
  • go.sum checksums are correctly updated, ensuring dependency integrity
  • No credentials, secrets, or sensitive data exposed

Recommendation: Consider verifying the changelog or release notes for v1.0.0-beta.9 to ensure there are no known security vulnerabilities or breaking changes that should be documented.

Performance & Resource Efficiency

Status: ℹ️ Not Applicable

  • This is a dependency version bump with no code logic changes
  • Performance impact depends entirely on what changed in beta.9 vs beta.8 (not visible in this PR)

🧪 Testing & Reliability

Status: ⚠️ Minor Observations

Observations:

  1. Good: The Docker upgrade test baseline is updated to v1.0.0-beta.9 (test/docker-e2e/upgrade_test.go:28)
  2. Question: The upgrade test now tests upgrading from beta.9 to the PR-built image. This is correct if beta.9 is the latest stable release.
  3. Missing: No evidence of running make deps or go mod tidy to ensure all transitive dependencies are properly resolved

Recommendations:

  • Run make deps (as mentioned in CLAUDE.md) to ensure all go.mod files are properly tidied
  • Consider running make test and make test-integration to verify the version bump doesn't introduce regressions
  • The CI checks (Buf) have passed, but broader test coverage would provide more confidence

📚 Documentation & Developer Experience

Status: ✅ Adequate for Chore PR

  • The PR title and description clearly state the purpose
  • For a dependency bump, this level of documentation is appropriate
  • The commit message is clear: "bump ev-node imports and docker upgrade test"

🎯 Specific File Reviews

go.mod files (5 files)

Files: apps/evm/single/go.mod, apps/grpc/single/go.mod, apps/testapp/go.mod, execution/grpc/go.mod, sequencers/single/go.mod

  • Change: github.com/evstack/ev-node v1.0.0-beta.8v1.0.0-beta.9
  • Assessment: ✅ Correct and consistent
  • Note: All files use the same version, which is good for consistency

go.sum files (2 files)

Files: execution/grpc/go.sum, sequencers/single/go.sum

  • Change: Hash updates for the new version
  • Assessment: ✅ Checksums properly updated
  • Old hash: h1:JxeVadKfk6TnFCEXYfYlBj8c7tQWyOxDECKy7PUgru4=
  • New hash: h1:9hRfT+EECIxrimhshjfAhER7x8wItFuytznC5jqRHrI=

test/docker-e2e/upgrade_test.go

File: test/docker-e2e/upgrade_test.go:28

  • Change: baseEVMSingleVersion = "v1.0.0-beta.8""v1.0.0-beta.9"
  • Assessment: ✅ Correct alignment with dependency version
  • Context: This constant defines the stable version from which the upgrade test starts
  • Good practice: Keeping the test baseline in sync with the latest stable release

🔄 Error Handling & Resilience

Status: ℹ️ Not Applicable

No error handling code is modified in this PR.

🤔 Questions & Recommendations

  1. Changelog Reference: Is there a changelog entry for this version bump? Consider linking to it in the PR description for reviewers to understand what changed in beta.9.

  2. Breaking Changes: Did v1.0.0-beta.9 introduce any breaking changes that might require code adjustments? The fact that only version numbers changed suggests backward compatibility, which is good.

  3. Verification Steps:

    # Recommended commands to run before merging:
    make deps           # Tidy all go.mod files
    make test           # Run unit tests
    make test-integration  # Run integration tests
    make lint           # Ensure linting passes
  4. CI Status: The Buf checks have passed. Consider waiting for/checking other CI workflows (tests, linting) before merging.

Final Verdict

LGTM (Looks Good To Me) with minor recommendations

This is a clean, well-executed dependency version bump. The changes are minimal, consistent, and appropriately scoped for a chore PR. The only recommendations are:

  1. Verify that make deps was run to ensure go.mod files are properly tidied
  2. Ensure full test suite passes (unit, integration, e2e)
  3. Consider adding a brief note about what changed in beta.9 for context

No blocking issues found. This PR is safe to merge once CI passes and the recommendations are addressed.


@tac0turtle tac0turtle enabled auto-merge November 4, 2025 15:02
@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.37%. Comparing base (271f74b) to head (9459fbb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2818   +/-   ##
=======================================
  Coverage   62.37%   62.37%           
=======================================
  Files          82       82           
  Lines        7304     7304           
=======================================
  Hits         4556     4556           
  Misses       2203     2203           
  Partials      545      545           
Flag Coverage Δ
combined 62.37% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tac0turtle tac0turtle added this pull request to the merge queue Nov 4, 2025
Merged via the queue into main with commit 2abf5a4 Nov 4, 2025
27 checks passed
@tac0turtle tac0turtle deleted the marko/single_docker-tests branch November 4, 2025 15:18
@github-project-automation github-project-automation bot moved this to Done in Evolve Nov 4, 2025
alpe added a commit that referenced this pull request Nov 7, 2025
* main:
  chore: docker prune - prevent deletion of unfound tags (#2819)
  refactor(reaping): use cache for seen hashes instead of in memory store (#2811)
  chore: bump ev-node imports and docker upgrade test (#2818)
  chore: update changlog (#2804)
  build(deps): Bump the all-go group across 3 directories with 2 updates (#2810)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants