Conversation
Replace Makefile with justfile for local development. CI workflows inline cargo/cross/docker commands directly to avoid adding just as a CI dependency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request migrates the project's build automation from GNU Make to the Justfile tool. The Makefile is removed entirely, replaced with a new Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Pipeline
participant Just as Justfile
participant Cross as Cross Compiler
participant Buildx as Docker Buildx
participant Registry as Registry
CI->>Just: just docker-build-push
Just->>Cross: cross build (x86_64-linux-gnu)
Cross-->>Just: dist/bin/linux/amd64
Just->>Cross: cross build (aarch64-linux-gnu)
Cross-->>Just: dist/bin/linux/arm64
Just->>Buildx: docker buildx build --push (multi-arch)
Buildx->>Buildx: linux/amd64, linux/arm64
Buildx->>Registry: push multi-arch image
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @tac0turtle's task —— View job Pull Request ReviewReviewing the migration from Makefile to justfile for local development. Review Tasks
Overall Assessment ✅This is a well-executed migration that successfully modernizes the build system while maintaining full feature parity. The changes are safe, beneficial, and properly implemented. Code Quality & Best Practices ✅Justfile Implementation:
Key Improvements over Makefile:
Feature Parity Analysis ✅I compared the deleted Makefile (171 lines) with the new justfile (163 lines) and confirmed 100% feature parity: Build targets: All preserved ( Documentation Updates ✅CLAUDE.md: Properly updated all build commands from CI Workflow Analysis ✅Smart CI Strategy: The PR correctly avoids adding Docker workflow (.github/workflows/docker.yml:79-89):
Release workflow (.github/workflows/release.yml:44):
Security Assessment ✅No security concerns identified:
Performance Considerations ✅Positive performance impact:
Areas of Excellence ✅
Minor Observations
Recommendation: ✅ APPROVEThis migration is ready to merge. It successfully modernizes the development experience while maintaining full backward compatibility in terms of available commands. The approach of keeping CI workflows dependency-free while improving local developer experience is well-balanced and practical. Key Benefits:
|
![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
🧹 Nitpick comments (1)
justfile (1)
135-136: Remove duplicate image tag argument indocker-build-push.The same tag is provided twice, which is redundant.
♻️ Proposed cleanup
docker buildx build --file ./Dockerfile.cross . \ --platform linux/amd64,linux/arm64 \ --tag {{docker_image}}:{{docker_tag}} \ - --tag {{docker_image}}:{{docker_tag}} \ --provenance=false \ --sbom=false \ --push🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@justfile` around lines 135 - 136, In the docker-build-push target remove the duplicated --tag argument: the lines that pass --tag {{docker_image}}:{{docker_tag}} twice should only include a single --tag; update the docker-build-push recipe so it only adds one --tag using the existing variables (docker_image and docker_tag) and remove the redundant duplicate --tag entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Line 44: The cargo build step currently runs RUSTFLAGS="-C target-cpu=native"
cargo build --profile maxperf --features jemalloc,asm-keccak --bin ev-reth which
omits the cross-compile target and uses the wrong profile; change that
invocation to include --target ${{ matrix.target }} and use --profile release
(or match whatever profile the packaging expects) so artifacts go to the matrix
target directory and the release profile path, and then update the subsequent
packaging steps that create the tarball/zip to point to the correct artifact
paths produced by the modified cargo build.
---
Nitpick comments:
In `@justfile`:
- Around line 135-136: In the docker-build-push target remove the duplicated
--tag argument: the lines that pass --tag {{docker_image}}:{{docker_tag}} twice
should only include a single --tag; update the docker-build-push recipe so it
only adds one --tag using the existing variables (docker_image and docker_tag)
and remove the redundant duplicate --tag entry.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.claude/skills/ev-reth-core.md.claude/skills/ev-reth-evolve.md.claude/skills/ev-reth-testing.md.github/workflows/docker.yml.github/workflows/release.ymlCLAUDE.mdMakefileREADME.mdjustfile
💤 Files with no reviewable changes (1)
- Makefile
|
|
||
| - name: Build release binary | ||
| run: make build-maxperf | ||
| run: RUSTFLAGS="-C target-cpu=native" cargo build --profile maxperf --features jemalloc,asm-keccak --bin ev-reth |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f=".github/workflows/release.yml"
echo "[Build command around line 44]"
rg -n 'Build release binary|cargo build' "$f" -A1
echo
echo "[Packaging directories - searching for profile paths]"
rg -n 'target/\$\{\{ matrix.target \}\}' "$f" -B1 -A1
echo
echo "[Matrix target configuration]"
rg -n 'target:' "$f" -A10 | head -30
echo
echo "[Full packaging section]"
sed -n '40,120p' "$f"Repository: evstack/ev-reth
Length of output: 2964
Build step no longer matches matrix target/profile artifact paths.
Line 44 builds without --target ${{ matrix.target }}, producing artifacts in the wrong location for cross-compilation. Additionally, --profile maxperf outputs to .../maxperf, while packaging steps expect .../release, causing the workflow to fail at the tarball/zip creation step.
🐛 Proposed fix
- run: RUSTFLAGS="-C target-cpu=native" cargo build --profile maxperf --features jemalloc,asm-keccak --bin ev-reth
+ run: RUSTFLAGS="-C target-cpu=native" cargo build --target ${{ matrix.target }} --profile maxperf --features jemalloc,asm-keccak --bin ev-rethThen update the packaging steps (lines 49 and 57):
- cd target/${{ matrix.target }}/release
+ cd target/${{ matrix.target }}/maxperf🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml at line 44, The cargo build step currently
runs RUSTFLAGS="-C target-cpu=native" cargo build --profile maxperf --features
jemalloc,asm-keccak --bin ev-reth which omits the cross-compile target and uses
the wrong profile; change that invocation to include --target ${{ matrix.target
}} and use --profile release (or match whatever profile the packaging expects)
so artifacts go to the matrix target directory and the release profile path, and
then update the subsequent packaging steps that create the tarball/zip to point
to the correct artifact paths produced by the modified cargo build.
1 participant
Replace Makefile with justfile for local development. CI workflows inline cargo/cross/docker commands directly to avoid adding just as a CI dependency.
Description
Type of Change
Related Issues
Fixes #(issue)
Checklist
Testing
Additional Notes
Summary by CodeRabbit
Documentation
Chores