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
ZocoLini
requested review from
QuantumExplorer,
llbartekll and
shumkov
as code owners
|
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 |
|
✅ Review complete (commit bc9bdf6) |
thepastaclaw
left a comment
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.
| # Map the requested base profile (dev|release) onto its iOS-tuned custom | ||
| # profile by appending "-ios" (dev-ios / release-ios) | ||
| PROFILE="${PROFILE}-ios" | ||
| # Map the requested base profile (dev|release) onto its swift-tuned custom |
There was a problem hiding this comment.
Let's move this one to scripts as well.
There was a problem hiding this comment.
that needs to be a different PR, its out of the scope of this one
shumkov
left a comment
shumkov
left a comment
There was a problem hiding this comment.
we move it to scripts folder in next PR
ZocoLini
force-pushed
the
chore/rename-script-profiles
branch
from
eea5960 to
68104ed
Compare
thepastaclaw
left a comment
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
528d478
Compare
![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 @.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
thepastaclaw
left a comment
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
ZocoLini
force-pushed
the
chore/rename-script-profiles
branch
from
528d478 to
bc9bdf6
Compare
thepastaclaw
left a comment
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.
3 participants
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