-
Notifications
You must be signed in to change notification settings - Fork 46
chore(llc): cleaned up dependencies and analyzer warnings #1060
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
Conversation
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 86 files out of 257 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughConverts GoogleSignIn DI registration to async initialization; updates login to prefer lightweight Google auth with fallback; migrates generated go_router wrappers from extensions to mixins and updates route classes; adds volume_controller plugin for desktop; bumps SDKs, dependencies, Gradle/Kotlin, and iOS deployment targets; assorted doc/changelog edits. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor AppStartup
participant DI as GetIt Locator
participant GSI as GoogleSignIn
Note over AppStartup,DI: DI registrations during init
AppStartup->>DI: registerSingletonAsync<GoogleSignIn>(...)
DI->>GSI: GoogleSignIn.instance.initialize(hostedDomain)
GSI-->>DI: initialized instance
DI-->>AppStartup: singleton ready (awaitAll)
sequenceDiagram
autonumber
actor User
participant UI as LoginScreen
participant GoogleSrv as GoogleService
participant Router as go_router (with $Route mixin)
User->>UI: tap "Sign in with Google"
UI->>GoogleSrv: attemptLightweightAuthentication()
alt returned account
GoogleSrv-->>UI: Account
UI->>Router: navigate (uses mixin .go/.push with typed extra)
else null or failure
UI->>GoogleSrv: authenticate() (fallback)
GoogleSrv-->>UI: Account or null
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing touches🧪 Generate unit tests
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1060 +/- ##
========================================
- Coverage 4.84% 4.83% -0.02%
========================================
Files 591 591
Lines 40168 40324 +156
========================================
+ Hits 1948 1951 +3
- Misses 38220 38373 +153 ☔ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dogfooding/lib/screens/login_screen.dart (1)
248-272: On web, replace the custom button with the official GSI renderButton.The web implementation doesn’t support
authenticate()and requires the SDK-rendered button. Consider returningrenderButton()frompackage:google_sign_in_web/web_only.dartwhenkIsWeb, and keeping the currentStreamButtonfor other platforms. This will require addinggoogle_sign_in_webto pubspec.Example (outside this hunk):
// at top: import 'package:google_sign_in_web/web_only.dart' show renderButton, GSIButtonConfiguration; // in build(): if (kIsWeb) { return renderButton(configuration: const GSIButtonConfiguration()); }Refs: Web plugin docs (authenticate unsupported; use renderButton), and the web-only
renderButtonAPI. (pub.dev)
♻️ Duplicate comments (1)
packages/stream_video_push_notification/android/settings.gradle (1)
21-23: Same AGP/Kotlin mismatch as the core module.This module also uses AGP 7.3.1 with Kotlin 2.1.0. Align with the chosen combination (e.g., AGP 8.8.0 + Kotlin 2.1.0) to avoid sync/build issues.
- id "com.android.library" version "7.3.1" apply false + id "com.android.library" version "8.8.0" apply falseVerify the root Gradle wrapper version meets AGP/Kotlin requirements.
🧹 Nitpick comments (10)
packages/stream_video_flutter/lib/src/screen_share/desktop_screen_selector.dart (2)
13-14: Polish doc formatting; consider code spans or member links.Wrap tokens in code spans for readability; if dartdoc can resolve member links, prefer
[StreamColorTheme.overlay]and[StreamTextTheme.tabBar].-/// Can be styled using overlay from [StreamColorTheme]; body, bodyBold and tabBar from [StreamTextTheme]. +/// Can be styled using `overlay` from [StreamColorTheme]; `body`, `bodyBold`, and `tabBar` from [StreamTextTheme].
1-1: Re-evaluate file-wide lint suppression.Since the comment references were cleaned up, the file-level
comment_referencesignore may no longer be needed. Remove it or scope it locally only if specific lines still trigger it.-// ignore_for_file: comment_referencespackages/stream_video/lib/stream_video.dart (1)
1-5: Nice top-level docs addition.Clear, concise description; no API changes. Consider linking to package site for discoverability (optional).
dogfooding/lib/screens/login_screen.dart (1)
282-287: Micro: avoid constructing Random per iteration.Minor perf/readability: create one
Randomand reuse.- for (var i = 0; i < size; i++) { - id += _alphabet[(math.Random().nextDouble() * 64).floor() | 0]; - } + final rng = math.Random(); + for (var i = 0; i < size; i++) { + id += _alphabet[(rng.nextDouble() * 64).floor()]; + }packages/stream_video/lib/src/ws/ws.dart (1)
54-55: Docs nit: capitalize “URL”.Tweak wording to “connection URL” for correctness.
-/// [WebSocketChannel] and accepts a connection url and an optional +/// [WebSocketChannel] and accepts a connection URL and an optionalpackages/stream_video_flutter/lib/stream_video_flutter.dart (1)
1-6: Helpful doc-header.Reads well; no API changes. Tiny grammar nit: “participant views” (singular “participant” as compound modifier).
-/// screens, participants views, controls, livestream UI, screen sharing, and +/// screens, participant views, controls, livestream UI, screen sharing, anddogfooding/pubspec.yaml (1)
11-44: Major bumps look aligned; please confirm platform/build implications
- Firebase (auth/core/crashlytics/messaging), go_router 16, get_it 8, google_sign_in 7, share_plus 11 are all breaking/behavioral changes across iOS/Android/Web. Verify minimum platform versions, Gradle/AGP settings, and router codegen are consistent.
- fl_chart pinned to 1.0.0 (not ^) is fine; just ensure transitive resolution doesn’t pull 1.1.x from other packages.
- mobile_scanner pre-release (rc) can be volatile—confirm dogfooding binaries still pass smoke tests.
Suggested checks:
- flutter pub get && dart pub outdated
- CI matrix: iOS, Android, Web, Windows, Linux smoke run (routing, auth, share, notifications).
Optional alignment:
- Consider aligning
httpto ^1.5.0 across workspace for fewer duplicate versions.Also applies to: 57-65
packages/stream_video/pubspec.yaml (1)
14-42: Deps bump LGTM; watch intl upper bound and http alignment
- Upper bound
intl <=0.21.0is okay, but keep an eye on consumers that might want >0.21. If feasible, test loosening when upstream allows.- Consider aligning
httphere to ^1.5.0 (dogfooding app already on ^1.5.0) to reduce tree duplication.dogfooding/lib/router/routes.dart (1)
21-21: Nit: make zero-field routesconst-constructible.Enables minor perf wins and clearer call sites.
-class HomeRoute extends GoRouteData with $HomeRoute { +class HomeRoute extends GoRouteData with $HomeRoute { + const HomeRoute(); @override Widget build(BuildContext context, GoRouterState state) { return const HomeScreen(); } } @@ -class LoginRoute extends GoRouteData with $LoginRoute { +class LoginRoute extends GoRouteData with $LoginRoute { + const LoginRoute(); @override Widget build(BuildContext context, GoRouterState state) { return const LoginScreen(); } }Also applies to: 30-30
packages/stream_video_flutter/android/build.gradle (1)
39-50: Deps update LGTM; consider aligning compileSdk with the example app.The example app uses compileSdk 35; this module is at 34. Not required, but aligning reduces warning noise and future API mismatch surprises.
Apply outside-this-range change:
- compileSdkVersion 34 + compileSdkVersion 35
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (26)
dogfooding/lib/di/injector.dart(1 hunks)dogfooding/lib/router/routes.dart(7 hunks)dogfooding/lib/router/routes.g.dart(2 hunks)dogfooding/lib/screens/login_screen.dart(1 hunks)dogfooding/linux/flutter/generated_plugin_registrant.cc(2 hunks)dogfooding/linux/flutter/generated_plugins.cmake(1 hunks)dogfooding/pubspec.yaml(2 hunks)dogfooding/windows/flutter/generated_plugins.cmake(1 hunks)packages/stream_video/lib/src/webrtc/media/camera_constraints.dart(1 hunks)packages/stream_video/lib/src/ws/ws.dart(1 hunks)packages/stream_video/lib/stream_video.dart(1 hunks)packages/stream_video/pubspec.yaml(2 hunks)packages/stream_video_flutter/android/build.gradle(1 hunks)packages/stream_video_flutter/android/settings.gradle(1 hunks)packages/stream_video_flutter/example/android/app/build.gradle(1 hunks)packages/stream_video_flutter/example/android/settings.gradle(1 hunks)packages/stream_video_flutter/example/pubspec.yaml(3 hunks)packages/stream_video_flutter/lib/src/screen_share/desktop_screen_selector.dart(1 hunks)packages/stream_video_flutter/lib/stream_video_flutter.dart(1 hunks)packages/stream_video_flutter/pubspec.yaml(1 hunks)packages/stream_video_noise_cancellation/android/build.gradle(0 hunks)packages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation.dart(1 hunks)packages/stream_video_push_notification/android/build.gradle(0 hunks)packages/stream_video_push_notification/android/settings.gradle(1 hunks)packages/stream_video_push_notification/lib/stream_video_push_notification.dart(1 hunks)packages/stream_video_push_notification/pubspec.yaml(1 hunks)
💤 Files with no reviewable changes (2)
- packages/stream_video_noise_cancellation/android/build.gradle
- packages/stream_video_push_notification/android/build.gradle
⏰ 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). (7)
- GitHub Check: analyze
- GitHub Check: build
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_screen_sharing
- GitHub Check: stream_video
🔇 Additional comments (18)
packages/stream_video_flutter/pubspec.yaml (1)
15-16: Version bumps look fine; verify workspace compatibility.Please confirm these constraints are compatible across the mono-repo (Dart SDK 3.6.2 / Flutter >= 3.27.4), and that no transitive breakages occur (e.g., intl upper bound with other deps).
Also applies to: 22-22
packages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation.dart (1)
1-4: Doc header LGTM.Clear, concise docs; no functional impact.
packages/stream_video_push_notification/lib/stream_video_push_notification.dart (1)
1-11: Library name uniqueness verified: no otherlibrary stream_video_push_notification;orpart of stream_video_push_notification;declarations found.packages/stream_video/lib/src/webrtc/media/camera_constraints.dart (1)
8-12: Doc correction LGTM.Accurately references MirrorMode defaults relative to FacingMode.
dogfooding/lib/di/injector.dart (1)
33-36: Verify GoogleSignIn initialization and add readiness wait
- Confirm that
GoogleSignIn.instance.initialize(hostedDomain: 'getstream.io')is a valid API withgoogle_sign_in: ^7.1.1; if it isn’t, switch to the constructor form:- locator.registerSingletonAsync<GoogleSignIn>(() async { - await GoogleSignIn.instance.initialize(hostedDomain: 'getstream.io'); - return GoogleSignIn.instance; - }); + locator.registerSingletonAsync<GoogleSignIn>(() async { + return GoogleSignIn(hostedDomain: 'getstream.io'); + });- In your
init()method, after registering all async singletons, add:to prevent any// Ensure async singletons (e.g., GoogleSignIn) are ready before use + await locator.isReady<GoogleSignIn>();GetItNotReadyErrorwhen resolvingGoogleSignIn.dogfooding/windows/flutter/generated_plugins.cmake (1)
21-22: Volume controller added on Windows — OKGenerated change looks consistent. Ensure
volume_controllerexists in pubspec and builds on Windows in CI.dogfooding/linux/flutter/generated_plugin_registrant.cc (1)
16-16: Volume controller registered on Linux — OKMatches the CMake plugin list. No manual edits needed; just keep generated files in sync with
flutter pub get.Also applies to: 40-43
dogfooding/lib/router/routes.g.dart (4)
22-23: Mixin-based factory migration LGTM.Factories now point to the mixins’
_fromState—consistent and readable. No issues spotted.Also applies to: 50-51, 79-80, 112-113, 146-147, 184-185, 219-220
166-178: Extra forwarding in helpers looks correct.
go/push/replaceconsistently forward_self.$extra. Matches the class fields.Also applies to: 201-213, 235-247, 94-106, 128-140
150-156: SDK constraint already enforces Dart 3+. pubspec.yaml specifiesenvironment.sdk: '>=3.0.0 <4.0.0', so record-typed extras are supported.
82-85: No missing or incorrectstate.extrausages found
All navigations toLobbyRoute,LivestreamRoute,CallRoute,CallParticipantsRoute, andCallStatsRouteuse the generated helpers with a correctly typed$extra, and there are no rawcontext.go/pushcalls to their string paths. The direct casts inroutes.g.dartare safe under current usage.dogfooding/lib/router/routes.dart (2)
21-26: Mixin adoption across routes LGTM.Clean switch to
$<Route>mixins; build methods unchanged. Looks good.Also applies to: 30-35, 39-60, 64-73, 77-94, 99-110, 114-125
80-85: Pubspec already aligned with Dart 3+ and go_router_builder version.
Both root and dogfooding pubspecs declare Dart 3 (root:>=3.0.0 <4.0.0, dogfooding:^3.6.2), and dogfooding usesgo_router_builder: ^4.0.1matching the code generator. No changes needed.packages/stream_video_flutter/example/android/app/build.gradle (1)
77-81: Stream Log bump looks good.The upgrade to io.getstream:stream-log:1.3.4 aligns with the library module and is safe.
Please run a release build to ensure R8 rules (if any) for stream-log remain compatible after the bump.
packages/stream_video_flutter/android/settings.gradle (1)
21-23: No change required: Kotlin 2.1.0 supports AGP 7.3.1–8.7.2 Kotlin Gradle plugin 2.1.0–2.1.10 is fully compatible with Android Gradle plugin versions 7.3.1 through 8.7.2, so the current pairing (AGP 7.3.1 + Kotlin 2.1.0) is valid (kotlinlang.org).packages/stream_video_flutter/example/android/settings.gradle (1)
21-24: Kotlin 2.1.0 + AGP 8.8.0: ensure Gradle wrapper compatibility.Config looks fine in isolation.
Confirm gradle/wrapper.properties uses a Gradle version supported by AGP 8.8.0 and Kotlin 2.1.0.
packages/stream_video_flutter/android/build.gradle (2)
39-50: Coroutines/ML Kit/libyuv updates: check transitive constraints.Nothing blocking seen, but please ensure:
- kotlinx-coroutines-core 1.10.2 is compatible with your Kotlin stdlib version resolved by Kotlin 2.1.0.
- ML Kit selfie segmentation beta may add keep rules; confirm R8 config if minification is enabled downstream.
39-50: Bump module minSdkVersion to 21
AndroidX libraries (appcompat 1.7.0 & core 1.16.0) now mandate API 21+ (default minSdk bumped in April 2024)—update yourdefaultConfig:- minSdkVersion 19 + minSdkVersion 21
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dogfooding/lib/screens/login_screen.dart (1)
97-116: Loading indicator leak on success; hide in finally.
showLoadingIndicatoris never hidden on success; movehideLoadingIndicatorto afinallyand remove fromcatch.try { await authController.login(user, environment); - } catch (e, _) { - if (mounted) { - hideLoadingIndicator(context); - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - duration: const Duration(seconds: 20), - content: Text('Error: $e'), - ), - ); - } - } + } catch (e, _) { + if (mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + duration: const Duration(seconds: 20), + content: Text('Error: $e'), + ), + ); + } + } finally { + if (mounted) hideLoadingIndicator(context); + }
♻️ Duplicate comments (2)
dogfooding/lib/screens/login_screen.dart (1)
41-58: Google Sign-In fallback looks good.Switch to lightweight auth with guarded fallback to
authenticate()addresses the prior no-op. Nice.packages/stream_video_flutter/example/pubspec.yaml (1)
17-19: Envied/Firebase majors – regenerate and add missing iOS plist.
- Regenerate Envied outputs for 1.x (
flutter pub run build_runner build) to producelib/env/env.g.dart.- Ensure
example/ios/Runner/GoogleService-Info.plistis present for Firebase iOS.Also applies to: 45-45
🧹 Nitpick comments (12)
packages/stream_video_flutter/example/ios/Podfile (1)
2-2: Lowering iOS minimum to 14 — verify repo-wide consistency and silence pod warnings.
- Sanity-check that all apps/targets (Runner.xcodeproj, AppFrameworkInfo.plist) and plugins support iOS 14 to avoid accidental SDK/API regressions.
- Optional: ensure Pods inherit 14.0 to prevent "deployment target" warnings.
Apply inside the existing post_install loop:
post_install do |installer| installer.pods_project.targets.each do |target| flutter_additional_ios_build_settings(target) + # Ensure Pods use iOS 14 minimum (prevents mixed-target warnings). + target.build_configurations.each do |config| + config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'] ||= '14.0' + end end end.github/workflows/legacy_version_analyze.yml (1)
6-6: Ensure “N-1” truly reflects your current baseline and stays in sync.If the project minimum is now 3.32.x, confirm that 3.29.0 is the intended N-1 and that the workflow skips packages that don’t support it. Consider sourcing this from a repo variable to avoid drift:
- flutter_version: "3.29.0" + flutter_version: ${{ vars.FLUTTER_N_MINUS_1 || '3.29.0' }}You can then update the variable without code changes. Use the verification script in melos.yaml comment to validate compatibility.
packages/stream_video_screen_sharing/pubspec.yaml (1)
20-23: Optional: align lints with Dart 3.6 toolchain.Upgrade to the latest lint set to catch newer rules and keep consistency with other packages using flutter_lints ^5.
dev_dependencies: flutter_test: sdk: flutter - flutter_lints: ^3.0.0 + flutter_lints: ^5.0.0packages/stream_video_flutter/pubspec.yaml (1)
15-16: Loosen or standardize intl constraint to reduce solve friction.
intl: ">=0.18.1 <=0.21.0"can block transitive upgrades; consider aligning to a caret (e.g.,^0.20.0) if compatible across the monorepo.- intl: ">=0.18.1 <=0.21.0" + intl: ^0.20.0Also applies to: 22-22
dogfooding/ios/Runner.xcodeproj/project.pbxproj (1)
586-586: Project-wide iOS 14.0 min – keep frameworks and Pods in lockstep.
- Good consistency across Runner and ScreenSharing.
- Recommend bumping Flutter/AppFrameworkInfo.plist MinimumOSVersion to 14.0 as well (currently 13.0 per PR notes) to avoid mismatched mins.
- Confirm Podfile targets for both app and extension are 14.0 and re-run
pod install.Also applies to: 716-716, 846-846, 971-971, 1093-1093, 1340-1340, 1461-1461, 1582-1582, 1702-1702, 1915-1915, 1963-1963
dogfooding/lib/screens/login_screen.dart (1)
254-263: Use the providedlabelprop (avoid hardcoded text).
GoogleLoginButtondefineslabelbut the build method hardcodes "Continue with Google".- return StreamButton.primary( - onPressed: onPressed, - label: 'Continue with Google', + return StreamButton.primary( + onPressed: onPressed, + label: label,Also applies to: 279-287
packages/stream_video_flutter/example/pubspec.yaml (1)
26-26: Dependency hygiene.Check that
intl ^0.20.0andshared_preferences ^2.5.3resolve cleanly with other workspace constraints; if not, consider pinning via resolution overrides in the example only.Also applies to: 29-29
dogfooding/pubspec.yaml (3)
26-26: get_it 8.x: ensure async registrations are awaited before first use.If using
registerSingletonAsync/registerLazySingletonAsync, ensureawait getIt.allReady()during app bootstrap.
30-30: http 1.5.x in app vs 1.1.x in packages: align constraints to reduce solver churn.Consider bumping internal packages that still declare
http: ^1.1.0to^1.5.0if compatible, to avoid confusing lockfile diffs.
41-41: New noise-cancellation package + local override: confirm all target platforms are wired.If used on desktop, ensure plugin registrant updates and runtime feature flags are correct. Keep the path override only in the mono-repo; remove before publishing the app externally.
Also applies to: 54-55
packages/stream_video/pubspec.yaml (2)
26-26: Prefer an exclusive upper bound for intl.Switch
<=0.21.0to<0.22.0to avoid inclusive-bound edge cases while still capping major changes.Apply within this file:
-intl: ">=0.18.1 <=0.21.0" +intl: ">=0.18.1 <0.22.0"
16-16: collection 1.19.x may tighten types; scan for now-ambiguous generics.Run analyzer with --fatal-infos to catch new implicit-dynamic issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
.github/workflows/legacy_version_analyze.yml(1 hunks)dogfooding/android/settings.gradle(1 hunks)dogfooding/ios/Flutter/AppFrameworkInfo.plist(1 hunks)dogfooding/ios/Runner.xcodeproj/project.pbxproj(12 hunks)dogfooding/lib/screens/login_screen.dart(1 hunks)dogfooding/pubspec.yaml(2 hunks)melos.yaml(1 hunks)packages/stream_video/CHANGELOG.md(1 hunks)packages/stream_video/pubspec.yaml(2 hunks)packages/stream_video_flutter/CHANGELOG.md(1 hunks)packages/stream_video_flutter/example/ios/Podfile(1 hunks)packages/stream_video_flutter/example/ios/Runner.xcodeproj/project.pbxproj(3 hunks)packages/stream_video_flutter/example/pubspec.yaml(2 hunks)packages/stream_video_flutter/pubspec.yaml(1 hunks)packages/stream_video_noise_cancellation/pubspec.yaml(1 hunks)packages/stream_video_push_notification/CHANGELOG.md(1 hunks)packages/stream_video_push_notification/pubspec.yaml(1 hunks)packages/stream_video_screen_sharing/pubspec.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- dogfooding/ios/Flutter/AppFrameworkInfo.plist
- packages/stream_video_flutter/CHANGELOG.md
- packages/stream_video_push_notification/CHANGELOG.md
- packages/stream_video/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_video_push_notification/pubspec.yaml
⏰ 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). (5)
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video
- GitHub Check: stream_video_screen_sharing
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video_flutter
🔇 Additional comments (13)
packages/stream_video_noise_cancellation/pubspec.yaml (1)
10-10: LGTM: Flutter minimum raised to >=3.32.0.Matches the repo-wide toolchain bump; no conflicts evident in this manifest.
Confirm CI uses a toolchain >=3.32.0 for jobs touching this package (build/test/pub dry-run).
packages/stream_video_screen_sharing/pubspec.yaml (1)
12-12: LGTM: Flutter minimum raised to >=3.32.0.Consistent with the rest of the repo.
packages/stream_video_flutter/example/ios/Runner.xcodeproj/project.pbxproj (1)
363-363: iOS min target bumped to 14 – verify ecosystem alignment.
- Ensure example/ios/Podfile
platform :ios, '14.0'is set (it is per PR), pods reinstalled, and CI/mac runners use Xcode 15+.- Keep AppFrameworkInfo.plist MinimumOSVersion consistent (ideally 14.0) to avoid mixed mins in frameworks.
Also applies to: 441-441, 490-490
packages/stream_video_flutter/pubspec.yaml (1)
11-11: Flutter SDK floor to 3.32.0 – confirm toolchain.Make sure CI and local contributors pin Flutter ≥3.32.0; otherwise pub get/analyzer will fail.
packages/stream_video_flutter/example/pubspec.yaml (1)
12-12: Flutter SDK floor to 3.32.0 – ensure example app CI matches.Align example workflow/tooling to Flutter ≥3.32.0 to avoid version drift with the packages.
dogfooding/pubspec.yaml (5)
17-21: Firebase major bumps: confirm platform setup and API migrations.Upgrades to firebase_auth/core/crashlytics/messaging are major; verify:
- Android: Firebase BOM/gradle plugin updated, messaging permission/notification channels intact.
- iOS/macOS: Pod versions resolved, Crashlytics initialization matches new APIs.
- Dart: initialization and auth exceptions adapted, background handlers for messaging re-registered.
21-21: fl_chart 1.x introduces API breaks—rebuild affected charts.Double-check axis/touch/legend API and theme changes to avoid runtime UI issues.
38-38: shared_preferences 2.5.x: verify migration off deprecated synchronous access.Make sure only async APIs are used; analyzer should be clean given the PR goal.
8-8: Confirmed Flutter min constraint bump across workspace. All pubspec.yaml files now declareflutter: ">=3.32.0".
27-27: Confirm go_router codegen and remove stale mixins
I ran the audit and only foundpart 'routes.g.dart'indogfooding/lib/router/routes.dart(noextensions_for_routeror manualRouteMixinusages). After bumping to go_router ^16.2.1, runflutter pub run build_runner build --delete-conflicting-outputsto regenerate your route mixins and delete any outdated generated files.
packages/stream_video/pubspec.yaml (3)
14-17: LGTM on dependency bumps; watch for subtle analyzer changes.Bumps to async/collection/equatable/fixnum/meta/pkg_info_plus/uuid/web_socket_channel look compatible with Dart 3.6; re-run analyzer on this package to catch type/nullable edge cases.
Also applies to: 20-21, 28-29, 39-41
15-17: Plus plugins on mobile/desktop: verify min platform versions and manifests.battery_plus/connectivity_plus updates may require newer Android/iOS platform mins or entitlements; confirm Gradle, Info.plist, and macOS/Linux manifests are aligned.
11-11: Flutter min version aligned across packages. Please verify your CI workflows (e.g..github/workflows/*) pin Flutter to 3.32.x.
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: 0
🧹 Nitpick comments (4)
melos.yaml (1)
15-17: Optional: pin a minimum minor in CI to avoid accidental downgrades.Since melos bootstrap permits any Flutter >=3.32.0 while CI runs 3.35.3 in some jobs, consider documenting the intended baseline (3.35.x) here or in CONTRIBUTING to reduce local/CI drift.
analysis_options.yaml (1)
3-4: Optional: prefertrailing_commas: allfor cleaner diffs.If the goal is consistent formatting and smaller diffs,
allenforces trailing commas in eligible lists/maps/params. Safe to adopt later if desired.-formatter: - trailing_commas: preserve +formatter: + trailing_commas: all.github/workflows/stream_video_flutter_workflow.yml (1)
45-50: Nit: duplicate step names.Two steps are named “Bootstrap Workspace” (one is actually a post-clean). Rename the first for clarity in job logs.
- - name: Bootstrap Workspace + - name: Post-clean Workspace run: melos postclean - - name: Bootstrap Workspace + - name: Bootstrap Workspace run: melos bootstrap --verboseAlso applies to: 48-50
dogfooding/pubspec.yaml (1)
57-65: Consider bumping build_runner to the latest 2.4.x.go_router_builder ^4 may leverage fixes in newer build_runner patch releases. Suggest ^2.4.12 to reduce codegen edge cases.
- build_runner: ^2.4.6 + build_runner: ^2.4.12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.github/workflows/app-distribute.yml(2 hunks).github/workflows/legacy_version_analyze.yml(1 hunks).github/workflows/stream_video_flutter_workflow.yml(1 hunks)analysis_options.yaml(1 hunks)dogfooding/pubspec.yaml(2 hunks)melos.yaml(1 hunks)packages/stream_video/CHANGELOG.md(1 hunks)packages/stream_video/pubspec.yaml(2 hunks)packages/stream_video_flutter/CHANGELOG.md(1 hunks)packages/stream_video_flutter/example/pubspec.yaml(2 hunks)packages/stream_video_flutter/pubspec.yaml(1 hunks)packages/stream_video_noise_cancellation/CHANGELOG.md(1 hunks)packages/stream_video_noise_cancellation/pubspec.yaml(1 hunks)packages/stream_video_push_notification/CHANGELOG.md(1 hunks)packages/stream_video_push_notification/pubspec.yaml(1 hunks)packages/stream_video_screen_sharing/CHANGELOG.md(1 hunks)packages/stream_video_screen_sharing/pubspec.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/stream_video_noise_cancellation/CHANGELOG.md
- packages/stream_video/CHANGELOG.md
- packages/stream_video_screen_sharing/CHANGELOG.md
- packages/stream_video_flutter/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/stream_video_noise_cancellation/pubspec.yaml
- .github/workflows/legacy_version_analyze.yml
- packages/stream_video_screen_sharing/pubspec.yaml
- packages/stream_video_flutter/pubspec.yaml
- packages/stream_video_push_notification/CHANGELOG.md
- packages/stream_video_push_notification/pubspec.yaml
- packages/stream_video_flutter/example/pubspec.yaml
- packages/stream_video/pubspec.yaml
⏰ 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). (8)
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: analyze
- GitHub Check: build
- GitHub Check: stream_video_screen_sharing
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video
- GitHub Check: analyze_legacy_version
🔇 Additional comments (10)
melos.yaml (1)
15-17: SDK/Flutter baseline alignment looks good.Dart ^3.8.0 and Flutter >=3.32.0 in melos bootstrap are consistent with the CI workflows bumped in this PR. No issues.
analysis_options.yaml (1)
3-4: Formatter config acknowledged.
trailing_commas: preserveis supported and harmless; formatter behavior remains stable across the repo..github/workflows/stream_video_flutter_workflow.yml (2)
5-5: Flutter 3.35.3 upgrade: OK.Matches other workflows and exceeds the repo’s min (>=3.32.0).
5-5: No Flutter SDK constraint conflicts detected. All pubspec.yaml “environment: flutter” constraints are ≤ 3.32.0 and ≤ 3.35.3, so both the legacy (3.32.0) and main (3.35.3) CI workflows will pass..github/workflows/app-distribute.yml (3)
32-32: Flutter 3.35.3 upgrade: OK.Aligned with the main workflow; should reduce cache misses and cross-job drift.
51-69: macOS runner/Xcode compatibility check (heads-up).Ensure the macOS-15 image has an Xcode compatible with Flutter 3.35.3 (it typically does). If build time spikes, pin Xcode via
xcode-selector the setup-ruby action’sxcode-version.
187-192: No-op formatting change acknowledged.
workingDir: dogfoodingis fine and matches the repo structure.dogfooding/pubspec.yaml (3)
6-9: Environment bump approved.Dart ^3.8.0 / Flutter >=3.32.0 aligns with CI and melos.
45-56: Overrides strategy looks correct.Local path overrides for the stream_video packages are consistent and will ensure dogfooding uses the workspace versions, including the new noise_cancellation package.
17-21: Verify platform/build prerequisites and run GoRouter codegen
- iOS: deployment target is set to 14.0 in dogfooding/ios/Podfile and Xcode project—this meets Firebase iOS SDK minimums.
- Android: compileSdkVersion 35 is declared in dogfooding/android/app/build.gradle; confirm the Android Gradle Plugin version in dogfooding/android/build.gradle (or settings) includes a compatible
com.android.tools.build:gradleentry for Firebase Messaging ^16.0.1.- Codegen: locally run
to ensure GoRouter ^16 codegen completes without errors.cd dogfooding dart run build_runner build --delete-conflicting-outputs
renefloor
left a comment
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.
Some nits, but good to see the project getting fully up to date again :)
|
@renefloor @Brazol
Can you use a newer version of this plugin? Google Play |
|
@Brazol thanks a lot, but, ehhm, it still doesn't support it. I gradle cleaned, flutter cleaned, but it still says that librenderscript-toolkit.so does not support 16kb page size. Before, I used to run |
Hey @tkortekaas thanks for the input. Our Android team is working on providing the fixed package for us. It will be ready before the deadline for sure. |
# Conflicts: # dogfooding/pubspec.yaml # packages/stream_video/CHANGELOG.md # packages/stream_video/lib/src/call/call_events.dart # packages/stream_video/lib/src/call/session/call_session.dart # packages/stream_video/lib/src/coordinator/open_api/event/open_api_event.dart # packages/stream_video/lib/src/models/call_participant_state.dart # packages/stream_video/lib/src/sfu/data/models/sfu_participant.dart # packages/stream_video/lib/src/sfu/sfu_extensions.dart # packages/stream_video/lib/src/sorting/call_participant_sorting_presets.dart # packages/stream_video/test/src/call/call_test.dart # packages/stream_video_flutter/CHANGELOG.md # packages/stream_video_flutter/example/pubspec.yaml # packages/stream_video_flutter/lib/src/call_participants/participant_label.dart # packages/stream_video_flutter/lib/src/theme/call_participant_theme.dart # packages/stream_video_noise_cancellation/CHANGELOG.md # packages/stream_video_push_notification/CHANGELOG.md # packages/stream_video_push_notification/pubspec.yaml # packages/stream_video_screen_sharing/CHANGELOG.md
resolves FLU-268
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores