compiles the Stellar CLI from source on every run - #502
Conversation
|
@Nemenwa Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThe pull request adds an npm override for the Stellar SDK, changes frontend CI to use ChangesRepository tooling
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/package.json`:
- Around line 38-41: Regenerate frontend/package-lock.json from the package.json
override for `@creit-tech/stellar-wallets-kit`, ensuring its nested
`@stellar/stellar-sdk` resolves to the overridden ^13.0.0 version and no longer
retains 16.0.1 or an invalid override state. Commit the updated lockfile without
changing the package manifest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cf56c9d-5242-4e62-9132-d9a7a9414f18
📒 Files selected for processing (1)
frontend/package.json
| "overrides": { | ||
| "@creit-tech/stellar-wallets-kit": { | ||
| "@stellar/stellar-sdk": "^13.0.0" | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
cd frontend
npm install --package-lock-only --ignore-scripts
npm ci --dry-run --ignore-scripts
npm ls --package-lock-only --all `@stellar/stellar-sdk`Repository: Stellar-Ecosystem/lodestar
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cd frontend
python3 - <<'PY'
import json
from pathlib import Path
pkg = json.loads(Path('package.json').read_text())
lock = json.loads(Path('package-lock.json').read_text())
print("overrides:")
print(json.dumps(pkg.get("overrides", {}), indent=2))
for name in ["node_modules/@creit-tech/stellar-wallets-kit", "node_modules/@creit-tech/stellar-wallets-kit/node_modules/@stellar/stellar-sdk", "node_modules/@stellar/stellar-sdk", "node_modules/@stellar/stellar-sdk/node_modules/@stellar/stellar-sdk"]:
p = name.lstrip("node_modules/")
print(f"\ndependency_tree[{p}]")
print(json.dumps(lock.get("packages", {}).get(name, {}), indent=2)[:1600])
def walk(path, prefix=""):
if path == "node_modules":
yield ""
else:
yield prefix + path.get("version") or ""
children = path.get("dependencies") or {}
for k in sorted(children):
yield from walk(children[k], prefix + k + "<-")
print("\nStellar SDK package entries:")
for k, v in sorted(lock.get("packages", {}).items()):
if k.endswith("/@stellar/stellar-sdk") or k == "node_modules/@stellar/stellar-sdk":
print(k, "=>", v.get("version"))
print("\nci dry exits ok")
PY
npm ls --package-lock-only --all `@stellar/stellar-sdk` 2>&1 | sed -n '1,80p'Repository: Stellar-Ecosystem/lodestar
Length of output: 4252
Regenerate the lockfile so the Stellar SDK override applies to the wallet kit.
frontend/package-lock.json still leaves @creit-tech/stellar-wallets-kit with its own nested @stellar/stellar-sdk@16.0.1 while the root resolution is 13.3.0, and --package-lock-only still leaves the overrides conflict as “invalid” in npm ls. Regenerate and commit the lockfile with the override resolved.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/package.json` around lines 38 - 41, Regenerate
frontend/package-lock.json from the package.json override for
`@creit-tech/stellar-wallets-kit`, ensuring its nested `@stellar/stellar-sdk`
resolves to the overridden ^13.0.0 version and no longer retains 16.0.1 or an
invalid override state. Commit the updated lockfile without changing the package
manifest.
- Add npm override to resolve peer dependency conflict between @creit-tech/stellar-wallets-kit and @trezor/connect-plugin-stellar - Switch frontend CI from npm install --legacy-peer-deps to npm ci for reproducible builds - Add .editorconfig for consistent formatting across Rust, JS/TS, YAML, and Markdown - Document dependency override resolution in README - Mention EditorConfig in CONTRIBUTING.md
|
Hi @Nemenwa, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
|
@ritik4ever I have resolved the conflict. Pleasereview. Thank you |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
5-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the opening maturity claim with the documented project status.
Line [5] calls Lodestar a “production-grade” project, while lines [13-18] describe it as early-stage and not production-grade. Remove or qualify “production-grade” so the README gives one clear maturity statement. Also write “open-source” as the compound adjective.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 5, Update the opening project description to remove or qualify the “production-grade” maturity claim so it matches the early-stage status documented elsewhere in the README. Also change “open source project” to “open-source project,” preserving the rest of the description.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CONTRIBUTING.md`:
- Line 68: Remove the duplicate Rust formatting bullet from the contributor
instructions, keeping the existing earlier statement that cargo fmt --all
--check runs in the contract-build CI job.
---
Outside diff comments:
In `@README.md`:
- Line 5: Update the opening project description to remove or qualify the
“production-grade” maturity claim so it matches the early-stage status
documented elsewhere in the README. Also change “open source project” to
“open-source project,” preserving the rest of the description.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 15e0afc8-5732-4e2f-a8a4-a094c1627972
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.editorconfig.github/workflows/ci.ymlCONTRIBUTING.mdREADME.mdfrontend/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/package.json
| - Rust: run `cargo fmt` before committing; `cargo fmt --all --check` runs in CI under the `contract-build` job | ||
| - We use Husky and `lint-staged` to automatically run Prettier, ESLint, and `cargo fmt` on staged files before every commit. | ||
| - In an emergency, you can skip these hooks by passing `--no-verify` to your git commit command: `git commit --no-verify -m "..."`. | ||
| - Rust: `cargo fmt --all --check` runs in CI under the `contract-build` job |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the duplicate Rust formatting bullet.
Line 65 already states that cargo fmt --all --check runs in the contract-build CI job. Remove Line 68 to keep the contributor instructions concise.
Proposed documentation cleanup
- - Rust: `cargo fmt --all --check` runs in CI under the `contract-build` job📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Rust: `cargo fmt --all --check` runs in CI under the `contract-build` job |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CONTRIBUTING.md` at line 68, Remove the duplicate Rust formatting bullet from
the contributor instructions, keeping the existing earlier statement that cargo
fmt --all --check runs in the contract-build CI job.
2 participants
Closes #398
closes #411
closes #416
closes #418
All acceptance criteria have been met:
Changes made:
@creit-tech/stellar-wallets-kitto use@stellar/stellar-sdk@^13.0.0instead of its bundled 16.x versionnpm install --legacy-peer-depstonpm cinpm installwith the overrideRoot cause:
@creit-tech/stellar-wallets-kitbundles@stellar/stellar-sdk@^16.0.0, but its dependency@trezor/connect-plugin-stellarrequires@stellar/stellar-sdk@^13.3.0. The override resolves this by forcing the wallets kit to use the project's SDK version (13.x).All files are staged and ready to commit.
Summary by CodeRabbit
Bug Fixes
Documentation
Chores