-
Notifications
You must be signed in to change notification settings - Fork 45
WIP refactor ios to swift package #47
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughRaised iOS minimum to 13.0 across plist, Podfile, and Xcode build configs. Integrated a local Swift package (FlutterGeneratedPluginSwiftPackage) into the Xcode project, removed the Pods embed script phase, and added a scheme PreActions script to run xcode_backend.sh prepare before builds. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Xcode as Xcode Build
participant Scheme as Build Scheme (dev)
participant Script as xcode_backend.sh (prepare)
participant SPM as Swift Package Resolver
participant Runner as Runner Target
Dev->>Xcode: Trigger Build
Xcode->>Scheme: Evaluate BuildAction
Scheme->>Script: PreActions: prepare
Script-->>Scheme: Prepared Flutter frameworks
Xcode->>SPM: Resolve FlutterGeneratedPluginSwiftPackage
SPM-->>Xcode: Provide package product
Xcode->>Runner: Compile/Link with package product
Runner-->>Dev: Build output (iOS 13+)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ 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 |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #47 +/- ##
===========================================
+ Coverage 72.03% 85.68% +13.65%
===========================================
Files 69 164 +95
Lines 1137 3549 +2412
===========================================
+ Hits 819 3041 +2222
- Misses 318 508 +190 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ios/Runner.xcodeproj/project.pbxproj (2)
17-18: Duplicate Pods_Runner.framework in FrameworksPods_Runner.framework is linked twice. This can cause duplicate linking warnings/errors.
Apply one of:
- Remove one entry via Xcode: Target > Build Phases > Link Binary With Libraries.
- Or adjust the pbxproj to keep only a single PBXBuildFile reference for Pods_Runner.framework.
Also applies to: 90-93
186-195: Restore missing CocoaPods embed/copy build phases
Withuse_frameworks!dynamic frameworks require both “[CP] Embed Pods Frameworks” and “[CP] Copy Pods Resources” in ios/Runner.xcodeproj/project.pbxproj to avoid runtime failures (dyld errors, missing bundles). Re-runpod installto re-integrate them or switch to static frameworks (use_frameworks! :linkage => :static) while retaining the Copy phase if pods include resources.
🧹 Nitpick comments (1)
ios/Runner.xcodeproj/xcshareddata/xcschemes/dev.xcscheme (1)
8-25: PreAction reliability: ensure FLUTTER_ROOT exists and apply across all schemes or move to a build phase
- $FLUTTER_ROOT may be unset in Xcode Scheme PreActions. Add a fallback to read it from Flutter/Generated.xcconfig, or convert this to a target build phase placed first.
- Only dev scheme has this PreAction. QA/prod schemes may fail. Prefer a target “Run Script” phase that runs before the current Flutter build phase to cover all schemes.
Suggested script content (fallback):
# Resolve FLUTTER_ROOT if missing if [ -z "$FLUTTER_ROOT" ] && [ -f "$SRCROOT/Flutter/Generated.xcconfig" ]; then FLUTTER_ROOT=$(sed -n -E 's/^FLUTTER_ROOT=(.*)$/\1/p' "$SRCROOT/Flutter/Generated.xcconfig") export FLUTTER_ROOT fi /bin/sh "$FLUTTER_ROOT/packages/flutter_tools/bin/xcode_backend.sh" preparePlease confirm dev/qa/prod builds succeed in Xcode and CI with this step available for each scheme.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ios/Flutter/AppFrameworkInfo.plist(1 hunks)ios/Podfile(1 hunks)ios/Runner.xcodeproj/project.pbxproj(16 hunks)ios/Runner.xcodeproj/xcshareddata/xcschemes/dev.xcscheme(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: App
- GitHub Check: Flutter Integration Tests (31)
🔇 Additional comments (3)
ios/Flutter/AppFrameworkInfo.plist (1)
23-25: MinimumOSVersion bump to 13.0 looks goodAligned with project configs. No issues.
ios/Runner.xcodeproj/project.pbxproj (2)
13-13: SPM package integration wired correctlyLocal Swift package references and dependencies are connected properly.
Ensure the ephemeral path Flutter/ephemeral/Packages/FlutterGeneratedPluginSwiftPackage exists before Xcode resolves packages (your “prepare” step should create it). Verify opening the project in Xcode doesn’t show a missing package prior to build.
Also applies to: 58-58, 90-91, 126-140, 200-202, 232-234, 1148-1161
404-404: Deployment target set to 13.0 across configsConsistent IPHONEOS_DEPLOYMENT_TARGET values. Good.
Also applies to: 491-491, 544-544, 655-655, 738-738, 906-906, 989-989, 1076-1076
| @@ -1,5 +1,5 @@ | |||
| # Uncomment this line to define a global platform for your project | |||
| # platform :ios, '12.0' | |||
| # platform :ios, '13.0' | |||
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.
🛠️ Refactor suggestion | 🟠 Major
Uncomment platform to enforce iOS 13 for all pods
Leaving this commented can cause pod deployment target drift and Xcode warnings. Set it explicitly.
Apply:
-# platform :ios, '13.0'
+platform :ios, '13.0'📝 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.
| # platform :ios, '13.0' | |
| platform :ios, '13.0' |
🤖 Prompt for AI Agents
In ios/Podfile around line 2, the iOS platform declaration is commented out
which can cause pods to inherit an unintended deployment target; uncomment and
set the platform line to explicitly enforce iOS 13.0 (e.g. change "# platform
:ios, '13.0'" to "platform :ios, '13.0'") so CocoaPods and Xcode use the correct
deployment target for all pods.
Summary by CodeRabbit