-
Notifications
You must be signed in to change notification settings - Fork 174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update devcontainer image: v1.0.0-rc.2 #2671
Conversation
0e74379
to
a2d2684
Compare
WalkthroughOhayo, sensei! This pull request updates the Docker image version across several configuration files related to a Rust development environment. The image version has been changed from Changes
Possibly related PRs
Suggested reviewers
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/ci.yml (2)
Line range hint
183-201
: Ohayo! Consider enhancing the test-hurl job robustnessThe new hurl testing setup is good, but could benefit from these improvements:
- Add a health check after starting katana
- Add error handling for the katana startup process
Consider this enhancement:
- run: | chmod +x /tmp/bins/katana chmod +x /tmp/bins/sozo nohup /tmp/bins/katana --dev --dev.accounts 2 --dev.no-fee & + KATANA_PID=$! + # Wait for katana to start + sleep 5 + if ! kill -0 $KATANA_PID 2>/dev/null; then + echo "Katana failed to start" + exit 1 + fi + # Simple health check + curl -s -X POST -H "Content-Type: application/json" \ + --data '{"jsonrpc":"2.0","method":"starknet_chainId","params":[],"id":1}' \ + http://localhost:5050 || { echo "Katana is not responding"; exit 1; }🧰 Tools
🪛 actionlint
32-32: label "ubuntu-latest-32-cores" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
Line range hint
89-104
: Consider expanding Docker environment validationThe current validation is good for katana, but consider adding:
- Validation for the sozo binary
- Explicit checks for required runtime dependencies
Here's a suggested enhancement:
- run: | chmod +x ./katana + chmod +x ./sozo + # Check for required runtime dependencies + for cmd in ldd curl; do + if ! command -v $cmd >/dev/null 2>&1; then + echo "Missing required dependency: $cmd" + exit 1 + fi + done + # Validate katana ./katana & KATANA_PID=$! sleep 2 if ! kill -0 $KATANA_PID; then echo "Katana exited with an error" exit 1 fi kill $KATANA_PID + # Validate sozo + if ! ./sozo --version; then + echo "Sozo validation failed" + exit 1 + fi🧰 Tools
🪛 actionlint
32-32: label "ubuntu-latest-32-cores" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
.devcontainer/devcontainer.json
(1 hunks).github/workflows/bench.yml
(2 hunks).github/workflows/ci.yml
(7 hunks).github/workflows/release-dispatch.yml
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- .devcontainer/devcontainer.json
- .github/workflows/bench.yml
- .github/workflows/release-dispatch.yml
🔇 Additional comments (2)
.github/workflows/ci.yml (2)
34-34
: Verify Docker image version consistency across jobs
The Docker image version has been updated to v1.0.0-rc.2
across multiple jobs. Let's ensure all jobs are using the same version.
Also applies to: 59-59, 126-126, 142-142, 157-157, 166-166, 175-175
✅ Verification successful
Docker image version is consistent across all jobs
All 7 jobs in the CI workflow are using the same Docker image version v1.0.0-rc.2
. No inconsistencies found, sensei!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Docker image version consistency
# Test: Search for all Docker image references. Expect: All should use v1.0.0-rc.2
rg 'image: ghcr.io/dojoengine/dojo-dev:' .github/workflows/ci.yml
Length of output: 438
34-34
: Ohayo sensei! The build artifact handling looks solid!
The separation of build and test concerns with artifact sharing is a good practice. The test job correctly downloads and sets up the binaries from the build job.
Let's verify the artifact handling setup:
Also applies to: 38-47
✅ Verification successful
Ohayo sensei! The artifact handling is perfectly synchronized!
The verification shows:
- Single upload point in the build job with
dojo-bins
artifact - Consistent downloads of
dojo-bins
across 5 different jobs - Proper path handling in both upload (
path: bins
) and downloads (path: /tmp/bins
)
The artifact workflow is well-structured and follows CI best practices with centralized binary building and distribution.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all jobs using dojo-bins artifact have proper download steps
# Test: Search for artifact downloads. Expect: Consistent artifact handling across jobs
rg -A 3 'uses: actions/download-artifact@v4' .github/workflows/ci.yml
Length of output: 617
Script:
#!/bin/bash
# Let's verify the upload artifact step to ensure it matches with downloads
rg -A 3 'uses: actions/upload-artifact@v4' .github/workflows/ci.yml
# Also check if the artifact name matches across upload/download
rg 'name: dojo-bins' .github/workflows/ci.yml
# Let's see the full build job to understand artifact creation
rg -A 20 'jobs:.*build:' .github/workflows/ci.yml
Length of output: 427
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2671 +/- ##
=======================================
Coverage 57.41% 57.42%
=======================================
Files 403 403
Lines 50818 50818
=======================================
+ Hits 29179 29182 +3
+ Misses 21639 21636 -3 ☔ View full report in Codecov by Sentry. |
Automated changes by create-pull-request GitHub action
Summary by CodeRabbit
New Features
test-hurl
job for improved testing capabilities.Bug Fixes
Documentation