Uh oh!
There was an error while loading. Please reload this page.
chore(swift-sdk): correctly refer to the swift sdk as swift in the build script and cargo profiles - #3949
chore(swift-sdk): correctly refer to the swift sdk as swift in the build script and cargo profiles#3949ZocoLini wants to merge 1 commit into
Conversation
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughRenames all iOS-specific identifiers to Swift equivalents: ChangesiOS → Swift profile and script rename
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
✅ Final review complete — no blockers (commit bc9bdf6) |
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Narrow rename PR that misses three in-tree call sites still invoking the deleted build_ios.sh: two GitHub Actions workflows (swift-sdk-release.yml, swift-example-app-ui-smoke.yml) and the local verify_build.sh helper. All three will fail outright on next run. Both agents converged on verify_build.sh; Claude additionally caught the two workflows and flagged stale references in docs/error messages, which the rename should sweep up.
🔴 3 blocking | 🟡 1 suggestion(s)
4 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.github/workflows/swift-sdk-release.yml`:
- [BLOCKING] .github/workflows/swift-sdk-release.yml:62: Release workflow still invokes the removed build_ios.sh
This PR renames packages/swift-sdk/build_ios.sh to build_swift.sh and removes the old path (no symlink). The 'Build DashSDKFFI.xcframework and install into Swift package' step still runs `bash packages/swift-sdk/build_ios.sh --target all --profile release`, so the next swift-sdk-release run will fail with `No such file or directory`. Update the invocation to the new script name.
In `.github/workflows/swift-example-app-ui-smoke.yml`:
- [BLOCKING] .github/workflows/swift-example-app-ui-smoke.yml:89: UI smoke workflow still invokes the removed build_ios.sh
The 'Build simulator DashSDKFFI.xcframework' step calls `bash packages/swift-sdk/build_ios.sh --target sim --profile dev`, but that script was renamed to build_swift.sh in this PR. The UI smoke job will fail on the next PR push that touches the swift-sdk area. Update the call to build_swift.sh.
In `packages/swift-sdk/verify_build.sh`:
- [BLOCKING] packages/swift-sdk/verify_build.sh:9: verify_build.sh still calls the removed build_ios.sh
verify_build.sh is a checked-in helper that drives the same xcframework build. After this rename, line 9 still executes `"$SCRIPT_DIR/build_ios.sh"`, which no longer exists, so running verify_build.sh fails immediately with `No such file or directory`. Point it at the new script.
In `packages/swift-sdk/SwiftExampleApp/Scripts/generate_bindings.sh`:
- [SUGGESTION] packages/swift-sdk/SwiftExampleApp/Scripts/generate_bindings.sh:48: Stale build_ios.sh references in helper scripts and onboarding docs
Several sibling scripts under SwiftExampleApp/Scripts/ and onboarding docs (BUILD_GUIDE_FOR_AI.md, BUILD_TROUBLESHOOTING.md, KeyWallet/README.md, rs-sdk-ffi READMEs/migration docs, CHANGELOG.md) still tell developers to run `./build_ios.sh` in error and usage messages. These don't fail at runtime, but they misdirect anyone following the printed instructions to a script that no longer exists. Worth sweeping in this same PR since the rename is the whole point.
Uh oh!
There was an error while loading. Please reload this page.
shumkov
left a comment
There was a problem hiding this comment.
we move it to scripts folder in next PR
eea5960 to
68104edCompare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
All three carried-forward runtime breakages are still valid at head 68104ed: the two workflows and verify_build.sh still invoke packages/swift-sdk/build_ios.sh even though this PR renames it to build_swift.sh and leaves no compatibility wrapper. The stale-reference follow-up is also still valid for Swift SDK-facing helper messages and docs, but it should be scoped to references made stale by this rename; I found no new latest-delta findings beyond the carried-forward items.
🔴 3 blocking | 🟡 1 suggestion(s)
4 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.github/workflows/swift-sdk-release.yml`:
- [BLOCKING] .github/workflows/swift-sdk-release.yml:62: Release workflow still calls the removed Swift build script
This PR renames packages/swift-sdk/build_ios.sh to packages/swift-sdk/build_swift.sh, and HEAD no longer tracks the old script. The swift-sdk release workflow still runs the deleted path, so the release job will fail with a missing script before it can build or package DashSDKFFI.xcframework.
In `.github/workflows/swift-example-app-ui-smoke.yml`:
- [BLOCKING] .github/workflows/swift-example-app-ui-smoke.yml:89: UI smoke workflow still calls the deleted build_ios.sh
The UI smoke workflow still builds the simulator framework through packages/swift-sdk/build_ios.sh. Since this PR removes that path and the replacement script is packages/swift-sdk/build_swift.sh, the workflow will fail before the SwiftExampleApp UI smoke tests can run.
In `packages/swift-sdk/verify_build.sh`:
- [BLOCKING] packages/swift-sdk/verify_build.sh:9: verify_build.sh invokes the deleted build script
verify_build.sh still executes "$SCRIPT_DIR/build_ios.sh", but HEAD only contains build_swift.sh. Running this checked-in verification helper now fails immediately with No such file or directory instead of building the Swift xcframework. The replacement also needs to pass a target because build_swift.sh exits with usage when no --target is supplied.
In `packages/swift-sdk/SwiftExampleApp/Scripts/generate_bindings.sh`:
- [SUGGESTION] packages/swift-sdk/SwiftExampleApp/Scripts/generate_bindings.sh:48: Swift SDK helper text still points at build_ios.sh
This helper still tells developers to run build_ios.sh after this PR removes that script. The same stale Swift SDK-facing references remain in sibling helper scripts and onboarding docs, including check_bindings_simple.sh, generate_bindings_minimal.sh, run_tests.sh's comment, BUILD_TROUBLESHOOTING.md, BUILD_GUIDE_FOR_AI.md, and the KeyWallet README. These are not CI blockers, but a rename PR should not leave current setup instructions pointing to a command that no longer exists.
68104ed to
528d478CompareThere 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 @.github/workflows/swift-sdk-release.yml:
- Line 62: Update the call to the old build script in
`.github/workflows/swift-example-app-ui-smoke.yml` where it currently invokes
`build_ios.sh --target sim --profile dev` to instead use the new
`build_swift.sh` script with the appropriate target and profile parameters to
maintain the same behavior. Ensure all references to the old build_ios.sh script
are migrated to the new build_swift.sh script across all CI workflow files to
prevent failures.
🪄 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
Run ID: 7cdc7b99-a6f7-416b-9aaf-2e8c0e56e629
📒 Files selected for processing (4)
.github/workflows/swift-sdk-release.ymlCargo.tomlpackages/swift-sdk/build_swift.shpackages/swift-sdk/run_tests.sh
Uh oh!
There was an error while loading. Please reload this page.
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Carried-forward review for PR #3949 (rename build_ios.sh → build_swift.sh, profile rename release-ios/dev-ios → release-swift/dev-swift). Verified against head 528d478: the prior swift-sdk-release.yml finding is now resolved, but three prior findings remain valid — the UI smoke workflow and verify_build.sh still invoke the deleted build_ios.sh (blocking), and several swift-sdk-owned helpers/docs still reference the deleted script (suggestion). No new defects introduced in the latest delta.
🔴 2 blocking | 🟡 2 suggestion(s)
4 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.github/workflows/swift-example-app-ui-smoke.yml`:
- [BLOCKING] .github/workflows/swift-example-app-ui-smoke.yml:89: UI smoke workflow still invokes deleted build_ios.sh
This workflow runs `bash packages/swift-sdk/build_ios.sh --target sim --profile dev` to build the simulator DashSDKFFI.xcframework, but this PR removes that script and replaces it with `packages/swift-sdk/build_swift.sh` (confirmed: build_ios.sh no longer exists in the tree). The UI smoke job will fail with a missing-file error at the build step before any XCUITest runs. The breakage is caused directly by the rename in this PR and must be addressed here.
In `packages/swift-sdk/verify_build.sh`:
- [BLOCKING] packages/swift-sdk/verify_build.sh:9: verify_build.sh executes deleted build_ios.sh
verify_build.sh still calls `"$SCRIPT_DIR/build_ios.sh"` (Step 1: Build/Install XCFramework), but that script is renamed to build_swift.sh in this PR. Running the checked-in verification helper at this head fails immediately with `No such file or directory` instead of building the xcframework. The replacement script also requires explicit --target/--profile arguments, so the invocation needs both renaming and arguments.
In `packages/swift-sdk/run_tests.sh`:
- [SUGGESTION] packages/swift-sdk/run_tests.sh:4: Stale comment in run_tests.sh still references build_ios.sh
Line 23 of this file was correctly updated to `bash build_swift.sh --target all --profile dev` in this PR, but the descriptive comment on line 4 in the same file still says it builds the sdk using `build_ios.sh`. Trivial to fix while touching this file and avoids future grep confusion.
In `packages/swift-sdk/SwiftExampleApp/Scripts/generate_bindings.sh`:
- [SUGGESTION] packages/swift-sdk/SwiftExampleApp/Scripts/generate_bindings.sh:48: Swift SDK helpers/docs still point developers at deleted build_ios.sh
Several swift-sdk-owned helpers and docs still print or document `build_ios.sh` in instructions to developers after this PR removes that script. Verified at head 528d4788:
- packages/swift-sdk/SwiftExampleApp/Scripts/generate_bindings.sh:48
- packages/swift-sdk/SwiftExampleApp/Scripts/generate_bindings_minimal.sh:53
- packages/swift-sdk/SwiftExampleApp/Scripts/check_bindings_simple.sh:28
- packages/swift-sdk/BUILD_GUIDE_FOR_AI.md:27, :108, :124, :147
- packages/swift-sdk/BUILD_TROUBLESHOOTING.md:21, :62
- packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/README.md:327
These tell developers to run a script that no longer exists. Not a runtime failure but ergonomic damage in code paths this PR directly invalidates. CHANGELOG.md:973 references historic PR #3666 and should not be rewritten; setup_ios_build.sh references a `rs-sdk-ffi/build_ios.sh` that did not exist before this PR — pre-existing and out of scope.
…ild script and cargo profiles
528d478 to
bc9bdf6Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Carried-forward verification at HEAD bc9bdf6: prior finding #1 (swift-example-app-ui-smoke workflow) is RESOLVED in this push. Prior #2 (verify_build.sh) and #3 (run_tests.sh comment) remain STILL VALID and are in-scope because they reference the renamed swift-sdk script directly. Prior #4 (generate_bindings*.sh / check_bindings_simple.sh) is dropped: those scripts point at packages/rs-sdk-ffi/build_ios.sh, which never existed in this repo and is not the script this PR renames — pre-existing stale references, not introduced or worsened by this PR. No new latest-delta defects found.
🔴 1 blocking | 💬 1 nitpick(s)
2 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/swift-sdk/verify_build.sh`:
- [BLOCKING] packages/swift-sdk/verify_build.sh:9: verify_build.sh still execs the deleted build_ios.sh
This PR renames packages/swift-sdk/build_ios.sh to build_swift.sh, but verify_build.sh — the canonical local helper for verifying the Swift/Rust FFI xcframework build — still calls "$SCRIPT_DIR/build_ios.sh" at line 9. After this PR merges, the script exits immediately with `No such file or directory` before xcodebuild ever runs, breaking the verification path the script exists to provide. The new script also requires an explicit --target, so a pure path rename would still fail; pass `--target all --profile dev` to preserve the previous default behavior.
QuantumExplorer
commented
Jul 23, 2026
I don't really see the point in this PR. |
This PR renames build_ios.sh to build_swift.sh and the dev-ios and profile-its profiles to *-swift. I think this better reflects what we are truly building, a swift-sdk. Everything still works the same. This change is just for consistency
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit