chore(samples): macos builds - #2865
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change centralizes credential persistence checks and excludes macOS. It also updates macOS Flutter configuration, links the generated Swift package, removes the CocoaPods resource-copy phase, and prepares Flutter before builds. ChangesCredential persistence policy
macOS Flutter build integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RunnerScheme
participant macos_assemble.sh
participant FlutterGeneratedPluginSwiftPackage
RunnerScheme->>macos_assemble.sh: Run prepare before build
macos_assemble.sh->>FlutterGeneratedPluginSwiftPackage: Prepare generated package
RunnerScheme->>FlutterGeneratedPluginSwiftPackage: Link package product
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
| import 'package:stream_chat_flutter/stream_chat_flutter.dart' hide PushProvider; | ||
| import 'package:stream_chat_persistence/stream_chat_persistence.dart'; | ||
|
|
||
| bool get platformSupportsPersistenceCredentials => !CurrentPlatform.isWeb && !CurrentPlatform.isMacOS; |
There was a problem hiding this comment.
But the package mentions MacOs as supported
https://pub.dev/packages/flutter_secure_storage
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 `@sample_app/lib/auth/auth_controller.dart`:
- Around line 13-14: Update tryAutoConnect() and debugReset() to use
platformSupportsPersistenceCredentials as the guard before reading or deleting
kStreamApiKey, kStreamUserId, and kStreamToken. Replace the web-only check in
tryAutoConnect() and apply the same shared guard to debugReset(), ensuring
unsupported macOS and web platforms never access these credentials.
🪄 Autofix
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: 1358ef21-1165-43ee-adb3-2858c76284be
📒 Files selected for processing (5)
sample_app/lib/auth/auth_controller.dartsample_app/macos/Flutter/Flutter-Debug.xcconfigsample_app/macos/Flutter/Flutter-Release.xcconfigsample_app/macos/Runner.xcodeproj/project.pbxprojsample_app/macos/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2865 +/- ##
=======================================
Coverage 72.87% 72.87%
=======================================
Files 429 429
Lines 27716 27716
=======================================
Hits 20199 20199
Misses 7517 7517 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Submit a pull request
Github Issue: related to #2862
CLA
Description of the pull request
The macos project files were a bit outdated. Also it is a bit tricky to use keychain storage on development builds, so I would just disable that for the sample app.
Summary by CodeRabbit
Bug Fixes
Build & Platform Updates