[ECO-5566] chore: update Android Gradle build to support the newer Flutter SDK#579
[ECO-5566] chore: update Android Gradle build to support the newer Flutter SDK#579
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdated Android and CI build tooling and configs: Gradle, AGP, Kotlin plugin and wrapper versions bumped; Java/Kotlin targets moved to Java 17; Firebase and desugaring dependencies adjusted; Flutter and Ruby tool versions updated; CI workflow now builds APK before driving tests and added compiler lint flags. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as GitHub Actions
participant Repo as Repository
participant Flutter as Flutter CLI
note right of CI #DDEEFF: Flutter integration workflow (android/ios)
CI->>Repo: checkout
CI->>Flutter: flutter pub get
CI->>Flutter: flutter build apk %% explicit new build step for Android
alt build success
CI->>Flutter: flutter drive --target=... %% instrumentation run after APK built
Flutter-->>CI: test results
else build failure
note right of CI #FEE: abort tests / report failure
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 |
aaf5852 to
f8073ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
android/build.gradle (1)
70-74: Replace afterEvaluate hook with configureEach for configuration-cache safety.Using projectsEvaluated/afterEvaluate makes builds less cacheable. Prefer configureEach.
Apply this diff:
-gradle.projectsEvaluated { - tasks.withType(JavaCompile) { - options.compilerArgs << "-Xlint:unchecked" << "-Xlint:deprecation" - } -} +tasks.withType(JavaCompile).configureEach { + options.compilerArgs += ["-Xlint:unchecked", "-Xlint:deprecation"] +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
android/build.gradle(3 hunks)
🔇 Additional comments (4)
android/build.gradle (4)
38-38: Good: use Flutter’s compileSdkVersion.This aligns the plugin with the app’s SDK level. Please ensure Flutter’s value resolves to 36 to meet the objective; override if Flutter is behind.
2-4: Module version set to 1.0-SNAPSHOT.Fine for local development. Verify it doesn’t unintentionally affect published artifacts or consumer versioning in your release pipeline.
28-35: Upgrade Firebase, desugar, and verify Ably versions
- Switch to the Firebase BOM (
platform("com.google.firebase:firebase-bom:<latest>")) and remove the hard-pinnedfirebase-messagingversion.- Bump
coreLibraryDesugaringtocom.android.tools:desugar_jdk_libs:2.xfor full Java 11+ API support.- Confirm you’re using the latest compatible
io.ably:ably-android:1.2.xrelease.
46-47: Java 17 is correct for AGP 8; no Kotlin plugin or sources underandroid/so nokotlinOptions.jvmTargetneeded. Consider adopting a Gradle toolchain for future robustness.
sacOO7
left a comment
There was a problem hiding this comment.
Build is failing with error
A problem occurred evaluating project ':ably_flutter'.
Could not get unknown property 'flutter' for extension 'android' of type com.android.build.gradle.LibraryExtension.
f8073ef to
63c86ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test_integration/android/settings.gradle (1)
21-23: Same Kotlin plugin version issue as noted in example/android/settings.gradleSee my earlier comment; we need to align this file with the corrected Kotlin plugin version as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
android/build.gradle(3 hunks)example/android/settings.gradle(1 hunks)test_integration/android/settings.gradle(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- 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). (4)
- GitHub Check: android
- GitHub Check: ios
- GitHub Check: android (24)
- GitHub Check: android (29)
63c86ab to
218b6c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
android/build.gradle(3 hunks)example/android/settings.gradle(1 hunks)test_integration/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)test_integration/android/settings.gradle(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- example/android/settings.gradle
- 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). (3)
- GitHub Check: android
- GitHub Check: android (29)
- GitHub Check: android (24)
218b6c2 to
13d991e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
android/build.gradle (1)
62-66: Compiler lint flags improve code quality feedback.Adding
-Xlint:uncheckedand-Xlint:deprecationwill surface potential type-safety and deprecation issues at compile time. However, note that these are warnings, not errors—the build will still succeed. Ensure the team is prepared to address these warnings or suppress them selectively with@SuppressWarningswhere justified.Consider converting these warnings to build errors (fail-on-warning) in CI to enforce code quality, or document the process for triaging and suppressing warnings:
gradle.projectsEvaluated { tasks.withType(JavaCompile) { options.compilerArgs << "-Xlint:unchecked" << "-Xlint:deprecation" // Optionally treat warnings as errors in strict mode: // options.compilerArgs << "-Werror" } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.github/workflows/flutter_integration.yaml(2 hunks)android/build.gradle(3 hunks)example/android/settings.gradle(1 hunks)test_integration/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)test_integration/android/settings.gradle(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- test_integration/android/settings.gradle
- example/android/settings.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). (5)
- GitHub Check: android
- GitHub Check: ios
- GitHub Check: android (29)
- GitHub Check: ios (iPhone 16)
- GitHub Check: android (24)
🔇 Additional comments (7)
test_integration/android/gradle/wrapper/gradle-wrapper.properties (1)
3-3: Gradle wrapper version aligns with test_integration, but verify AGP compatibility in android/build.gradle.The upgrade to 8.10.2 is reasonable, but ensure it matches the Android Gradle Plugin (AGP) version. Per the official AGP/Gradle compatibility matrix, AGP 8.12.x requires Gradle 8.13+; Gradle 8.10.2 is compatible with AGP 8.8.x. The mismatch will cause build failures—see review of android/build.gradle line 13.
.github/workflows/flutter_integration.yaml (2)
13-13: iOS device update is reasonable; validate availability.Updating to iPhone 16 is fine, but verify that
macos-latestrunner image includes this simulator. If not, the workflow will fail with a simulator lookup error. The latest macOS runners typically support current-year iPhone models, so this should be safe.
36-39: Pod install step is appropriate for CocoaPods-based iOS dependencies.The added
pod installstep beforeflutter driveensures iOS dependencies are available. This aligns with modern iOS build practices and the broader Android/iOS modernization in this PR.android/build.gradle (4)
38-38: Dynamic compileSdk binding is a best practice.Using
flutter.compileSdkVersionensures the plugin builds against the same SDK as the Flutter framework, reducing version mismatches and future-proofing the plugin. This aligns with modern Flutter best practices.
2-4: Version comment is clear and helpful.The snapshot suffix comment accurately conveys that this is a development build. The comment is well-placed and improves maintainability.
34-34: Desugaring is likely necessary withminSdkVersion 19, but verify if SDK minimum will be increased.Your codebase doesn't directly use Java 8+ APIs (no lambdas, streams, or Optional found), but
firebase-messaging 23.0.4almost certainly uses them internally. SinceminSdkVersion 19(API level 19, 2013) predates Java 8 native support (added in API 24, 2016), desugaring is required to run these dependencies on older devices.Removing
coreLibraryDesugaringwould likely cause build or runtime failures unlessminSdkVersionis increased (e.g., to 24+). If there are concrete plans to increase the minimum SDK as part of ongoing modernization, this becomes an optional refactor. Otherwise, keep the dependency.
46-47: Java 17 upgrade is verified as compatible with declared dependencies.Verification confirms that both
ably-android:1.2.40(requires Java 8+) andfirebase-messaging:23.0.4(Android library with no JDK blockers) support Java 17. The Android Gradle Plugin 8.12.1 actually requires JDK 17, anddesugar_jdk_libs:1.1.5is already configured to ensure Android API compatibility for minSdk 19. No reflection, compilation, or dependency resolution errors are expected.
13d991e to
1281524
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test_integration/android/settings.gradle (1)
23-23: Bumpgoogle-servicesplugin for AGP 8.7 compatibility.
com.google.gms.google-services4.3.10 is incompatible with Android Gradle Plugin 8.x and will cause build failures due to removedBaseVariantAPIs. Update to version 4.4.0 or later.Apply this diff:
- id "com.google.gms.google-services" version "4.3.10" apply false + id "com.google.gms.google-services" version "4.4.2" apply false
🧹 Nitpick comments (1)
test_integration/ios/Runner.xcodeproj/project.pbxproj (1)
345-345: Consider raising deployment target for consistency.The test_integration project's deployment target remains at iOS 10.0, while the example project was raised to iOS 12.0 (at project level). For consistency and to match modern iOS support requirements, consider updating this project's deployment target as well.
#!/bin/bash # Verify deployment targets across both iOS projects echo "=== Example project deployment targets ===" rg -n "IPHONEOS_DEPLOYMENT_TARGET" example/ios/Runner.xcodeproj/project.pbxproj | head -10 echo -e "\n=== Test integration project deployment targets ===" rg -n "IPHONEOS_DEPLOYMENT_TARGET" test_integration/ios/Runner.xcodeproj/project.pbxproj | head -10Also applies to: 431-431, 480-480
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
test_integration/ios/Podfile.lockis excluded by!**/*.locktest_integration/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.github/workflows/flutter_integration.yaml(2 hunks).tool-versions(1 hunks)android/build.gradle(3 hunks)example/android/app/build.gradle(1 hunks)example/android/settings.gradle(1 hunks)example/ios/Flutter/AppFrameworkInfo.plist(1 hunks)example/ios/Runner.xcodeproj/project.pbxproj(7 hunks)example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme(2 hunks)example/ios/Runner/Info.plist(1 hunks)test_integration/android/app/build.gradle(1 hunks)test_integration/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)test_integration/android/settings.gradle(1 hunks)test_integration/ios/Flutter/AppFrameworkInfo.plist(1 hunks)test_integration/ios/Runner.xcodeproj/project.pbxproj(4 hunks)test_integration/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme(2 hunks)test_integration/ios/Runner/AppDelegate.swift(1 hunks)test_integration/ios/Runner/Info.plist(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test_integration/ios/Flutter/AppFrameworkInfo.plist
🚧 Files skipped from review as they are similar to previous changes (2)
- test_integration/android/gradle/wrapper/gradle-wrapper.properties
- example/android/settings.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). (5)
- GitHub Check: android (29)
- GitHub Check: ios (iPhone 16)
- GitHub Check: android (24)
- GitHub Check: ios
- GitHub Check: android
🔇 Additional comments (26)
example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme (2)
3-3: LGTM: Xcode version updated to 15.1.The LastUpgradeVersion bump from "1300" (Xcode 13.0) to "1510" (Xcode 15.1) aligns with keeping the project configuration current with modern tooling.
51-51: LGTM: GPU validation enabled for debugging.Enabling GPU validation mode helps catch Metal API usage errors and performance issues during development. This is a helpful debugging feature with no impact on release builds.
example/ios/Runner/Info.plist (1)
52-55: LGTM: Modern iOS capability flags added.The two new keys enable contemporary iOS features:
CADisableMinimumFrameDurationOnPhone: Allows the app to utilize high refresh rate displays (ProMotion)UIApplicationSupportsIndirectInputEvents: Enables indirect input device support (trackpad, Apple Pencil hover)These are opt-in enhancements with no negative impact on older devices.
test_integration/ios/Runner/Info.plist (1)
44-47: LGTM: Consistent capability flags with example app.The same modern iOS capability flags are appropriately added to the integration test app, maintaining consistency with the example app configuration.
test_integration/ios/Runner/AppDelegate.swift (1)
4-4: LGTM: Modernized Swift entry point annotation.The change from
@UIApplicationMainto@mainadopts the Swift 5.3+ standard attribute for marking the app's entry point. This is functionally equivalent and aligns with current Swift best practices..github/workflows/flutter_integration.yaml (2)
13-13: LGTM: Updated to iPhone 16 simulator.Using the iPhone 16 simulator keeps the integration tests running on a current device model. The comment correctly notes that iOS version isn't specified since it changes frequently.
35-39: No changes required—directory navigation is correct.The workflow correctly executes
flutter drivefrom thetest_integration/directory. After runningpod installintest_integration/ios, thecd ..command returns totest_integration/, which is the correct location to runflutter drive(the directory containingpubspec.yamlfor the integration test package). The navigation logic is sound and requires no adjustment.Likely an incorrect or invalid review comment.
example/ios/Runner.xcodeproj/project.pbxproj (5)
6-6: LGTM: Object version updated for Xcode 14+ compatibility.Bumping objectVersion from 50 to 54 ensures compatibility with Xcode 14 and later toolchains.
160-160: LGTM: Project upgraded to Xcode 15.1.LastUpgradeCheck updated from 1300 (Xcode 13.0) to 1510 (Xcode 15.1), consistent with the scheme file changes.
206-211: LGTM: Build script phase improvements.Adding
alwaysOutOfDate = 1ensures Flutter's thin binary script runs every build (necessary for Flutter tooling), and including${TARGET_BUILD_DIR}/${INFOPLIST_PATH}in inputPaths helps Xcode properly track dependencies.
252-252: LGTM: Flutter build script marked to run every build.Setting
alwaysOutOfDate = 1for the Flutter build script ensures it executes on every build, which is required for Flutter's incremental build system.
362-362: Verify whether the Runner target should be updated to match the project-level iOS 12.0 deployment target.The PR successfully updated project-level configurations (Release, Debug, Profile) from 9.0 to 12.0. However, the Runner target's build settings at lines 388, 528, and 564 still retain
IPHONEOS_DEPLOYMENT_TARGET = 10.0, which overrides the project-level setting. Additionally, the Podfile (line 44) enforces 10.0 for generated pods. In Xcode's build settings hierarchy, target-level settings take precedence, so the effective deployment target for the Runner app remains iOS 10.0, not 12.0.Confirm whether this mixed configuration is intentional or if all deployment targets should be unified at 12.0:
- If intentional: document why Runner must support iOS 10.0 while the project targets 12.0
- If unintentional: update lines 388, 528, 564 and the Podfile to 12.0 for consistency
test_integration/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme (1)
3-3: LGTM: Consistent Xcode upgrade with example project.The test_integration scheme receives the same updates as the example project: Xcode 15.1 upgrade and GPU validation mode enabled. This consistency is appropriate.
Also applies to: 53-53
test_integration/ios/Runner.xcodeproj/project.pbxproj (3)
6-6: LGTM: Object version updated for modern Xcode.ObjectVersion bumped from 51 to 54, aligning with the example project's upgrade to Xcode 14+ compatibility.
158-158: LGTM: Project upgraded to Xcode 15.1.LastUpgradeCheck consistently updated to 1510 (Xcode 15.1) across both example and test_integration projects.
202-207: LGTM: Build script phases updated consistently.The Thin Binary and Run Script phases receive the same
alwaysOutOfDate = 1setting and input path tracking as the example project, ensuring Flutter tooling works correctly.Also applies to: 218-218
.tool-versions (1)
1-1: LGTM! Ruby version updated to current stable release.The Ruby version bump from 3.0.1 to 3.4.5 is appropriate as Ruby 3.0.x reached end-of-life in March 2024.
test_integration/android/app/build.gradle (1)
14-20: LGTM! Java 17 toolchain upgrade is correct.The Java and Kotlin compatibility settings are appropriately upgraded to Java 17, aligning with the project-wide toolchain modernization.
example/android/app/build.gradle (2)
15-21: LGTM! Java 17 toolchain upgrade is correct.The Java and Kotlin compatibility settings are appropriately upgraded to Java 17, consistent with the project-wide toolchain modernization.
52-52: ThecoreLibraryDesugaringdependency is indeed unnecessary and can be safely removed.Verification confirms:
- Java 17 is explicitly configured (
JavaVersion.VERSION_17)- The example app itself contains no usage of desugared APIs
- All declared dependencies (junit, androidx.test, espresso) are compatible with Java 17 and don't require desugaring
- Java 17 natively provides APIs that previously required desugaring (java.time, java.util.stream, java.util.function, Optional)
Recommend removing the
coreLibraryDesugaringdependency from line 52 since it serves no purpose with Java 17.android/build.gradle (5)
2-4: LGTM! Version and comment clarification.The version comment appropriately clarifies that this is a development build artifact not intended for external distribution.
38-38: LGTM! Dynamic compileSdkVersion aligns with Flutter SDK.Using
flutter.compileSdkVersionensures the plugin stays synchronized with Flutter's SDK requirements, addressing the PR objective to updatecompileSdkVersion.
46-48: LGTM! Java 17 compatibility upgrade is correct.The Java source and target compatibility upgrade to Java 17 aligns with modern Android development requirements and the project-wide toolchain modernization.
62-66: LGTM! Compiler lint flags improve code quality.Enabling
-Xlint:uncheckedand-Xlint:deprecationwill surface warnings about unchecked operations and deprecated API usage, improving code maintainability.
34-34: Desugaring is still necessary and should be maintained.Java 17 as the compile target (lines 46-47) does not affect runtime API availability. With
minSdkVersion 19(Android 4.4), theably-android:1.2.40dependency requires desugaring because it uses Java 8+ library APIs (java.time,java.util.stream,java.util.function,Optional) that are only natively available on Android API 26+. ThecoreLibraryDesugaring 'com.android.tools:desugar_jdk_libs:1.1.5'configuration is correct.test_integration/android/settings.gradle (1)
21-22: LGTM! AGP and Kotlin plugin versions updated appropriately.The Android Gradle Plugin (8.7.0) and Kotlin Android plugin (2.2.20) versions are updated to support modern Android development tooling and align with the Java 17 upgrade.
1281524 to
2be4e19
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
example/android/settings.gradle (1)
22-22: Kotlin 2.2.20 is now published and available.The previous review comment flagged this version as non-existent, but Kotlin 2.2.20 is a published release available on the Gradle plugins portal. This version is appropriate and the change is good.
test_integration/android/settings.gradle (1)
21-23:⚠️ Upgradegoogle-servicesplugin for AGP 8.7 compatibility (duplicate of past review).The latest release of the google-services plugin for Android (v4.4.3) is now available. The version 4.3.10 in use predates AGP 8.x and may cause compatibility issues. The past review flagged this concern; upgrade to a 4.4.x release (e.g., 4.4.2 or 4.4.3) for proper AGP 8.7 compatibility.
Apply this diff:
- id "com.google.gms.google-services" version "4.3.10" apply false + id "com.google.gms.google-services" version "4.4.2" apply false
🧹 Nitpick comments (2)
.github/workflows/flutter_integration.yaml (1)
15-15: Verify macOS 13 is appropriate for the updated toolchain.Pinning to
macos-13(released 2022) may be overly restrictive given the broader toolchain upgrades in this PR (Gradle 8.12.1, Java 17). Confirm that:
- macOS 13 is the minimum version required by the new Flutter/Gradle/Java 17 stack, or
- A newer macOS version (e.g.,
macos-14ormacos-15-xl) would be compatible and preferable for sustainability.Note: The comment at line 13 indicates iOS version is intentionally unspecified to accommodate changes, but the runner version is now hardcoded.
Consider documenting the rationale for this specific macOS version choice, especially if it's a hard minimum requirement imposed by the new toolchain.
android/build.gradle (1)
34-34:⚠️ Desugar library version inconsistency across modules.This file uses
desugar_jdk_libs:1.1.5, whiletest_integration/android/app/build.gradleusesdesugar_jdk_libs:2.0.3. For a coherent Java 17 migration across the project, align both modules to use the same (preferably latest stable) version.Consider updating
android/build.gradleto match:- coreLibraryDesugaring 'com.android.tools:desugar_jdk_libs:1.1.5' + coreLibraryDesugaring 'com.android.tools:desugar_jdk_libs:2.0.3'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
test_integration/ios/Podfile.lockis excluded by!**/*.locktest_integration/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.github/workflows/flutter_integration.yaml(2 hunks).tool-versions(1 hunks)android/build.gradle(3 hunks)example/android/app/build.gradle(1 hunks)example/android/settings.gradle(1 hunks)example/ios/Flutter/AppFrameworkInfo.plist(1 hunks)example/ios/Runner.xcodeproj/project.pbxproj(7 hunks)example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme(2 hunks)example/ios/Runner/Info.plist(1 hunks)test_integration/android/app/build.gradle(2 hunks)test_integration/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)test_integration/android/settings.gradle(1 hunks)test_integration/ios/Flutter/AppFrameworkInfo.plist(1 hunks)test_integration/ios/Runner.xcodeproj/project.pbxproj(4 hunks)test_integration/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme(2 hunks)test_integration/ios/Runner/AppDelegate.swift(1 hunks)test_integration/ios/Runner/Info.plist(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- example/android/app/build.gradle
- test_integration/ios/Runner.xcodeproj/project.pbxproj
- test_integration/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
- .tool-versions
- test_integration/ios/Runner/AppDelegate.swift
- test_integration/android/gradle/wrapper/gradle-wrapper.properties
- example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
- example/ios/Flutter/AppFrameworkInfo.plist
⏰ 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: android (29)
- GitHub Check: ios (iPhone 15)
- GitHub Check: android (24)
- GitHub Check: android
- GitHub Check: ios
🔇 Additional comments (11)
.github/workflows/flutter_integration.yaml (1)
34-36: Verify the flutter drive command works from the repository root.The command changed from
cd test_integration && flutter drivetoflutter drive. This assumes the test driver can locate or auto-discover the integration test target from the root directory.Please confirm:
- Does
flutter drivenow correctly discover/run tests from thetest_integrationdirectory without explicit navigation, or- Is there a
pubspec.yamlor test configuration at the root that defines this behavior?This is a structural change that could silently fail or run incorrect tests if misconfigured.
test_integration/android/app/build.gradle (1)
14-22: Java 17 configuration looks solid.The Java 17 compilation targets are properly aligned across
compileOptions,kotlinOptions.jvmTarget, and desugaring is enabled to support core library APIs on older Android versions. Thedesugar_jdk_libs:2.0.3dependency is appropriate for this setup.android/build.gradle (3)
2-4: Documentation comments are clear.The version identifier and context comments improve clarity about this file's role in the build. No functional changes needed.
46-47: Java 17 alignment is correct.The Java 17 compilation targets align with the PR objectives and are consistent with the test_integration configuration.
62-66: Lint compiler args enhance code quality feedback.Adding
-Xlint:uncheckedand-Xlint:deprecationflags is a good practice for surfacing potential code quality issues during compilation. This aids long-term maintainability.test_integration/android/settings.gradle (1)
22-22: Kotlin 2.2.20 is confirmed published and available.The version is appropriate; no issue here. (Note: The past review comment on the example/android/settings.gradle file flagged this as non-existent, but Kotlin 2.2.20 is a published release available on the Gradle plugins portal.)
example/ios/Runner.xcodeproj/project.pbxproj (3)
6-6: LGTM: Xcode version metadata updates are appropriate.The
objectVersion = 54andLastUpgradeCheck = 1510updates correspond to Xcode 15.1 and are consistent with modernizing the iOS build tooling.Also applies to: 160-160
206-206: LGTM: Build phase improvements for Flutter tooling.Adding
alwaysOutOfDate = 1ensures the Thin Binary script runs on every build, which is appropriate for Flutter'sembed_and_thinscript. The additional input path improves Xcode's dependency tracking.Also applies to: 211-211
252-252: LGTM: Build phase improvement for Flutter build script.Adding
alwaysOutOfDate = 1ensures Flutter's build script runs on every build, which is necessary for proper Flutter integration.test_integration/ios/Runner/Info.plist (1)
44-47: LGTM: Modern iOS capability keys added.The two new Info.plist keys are appropriate for supporting modern iOS features:
CADisableMinimumFrameDurationOnPhone: Controls ProMotion display frame rate behaviorUIApplicationSupportsIndirectInputEvents: Required for proper indirect input support (iOS 13.4+)These additions align with modernizing iOS platform support and are consistently applied across both example and test_integration configurations.
example/ios/Runner/Info.plist (1)
52-55: LGTM: Consistent iOS capability keys added.The same modern iOS capability keys added to the test_integration configuration have been applied here, ensuring consistency across all iOS app configurations in the repository.
2be4e19 to
662dd7e
Compare
500c319 to
a4544d0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
test_integration/android/settings.gradle (2)
21-21: Critical: AGP version mismatch between modules.The Android Gradle Plugin version is set to 8.7.0 here, but
android/build.gradleuses 8.12.1. This inconsistency can cause dependency resolution conflicts, build failures, and subtle runtime issues. All Android modules must use the same AGP version.Apply this diff to align with the main module:
- id "com.android.application" version "8.7.0" apply false + id "com.android.application" version "8.12.1" apply falseNote: The past review comment indicates this was "addressed in commit 500c319", but the code still shows version 8.7.0. Please verify this fix was applied correctly.
23-23: Critical: Incompatible google-services plugin version with AGP 8.x.The
com.google.gms.google-servicesplugin version 4.3.10 predates AGP 8.x and will cause build crashes due to removedBaseVariantAPIs. Google's compatibility documentation requires at least version 4.4.x for AGP 8.x.Apply this diff to use a compatible version:
- id "com.google.gms.google-services" version "4.3.10" apply false + id "com.google.gms.google-services" version "4.4.2" apply falseNote: The past review comment indicates this was flagged previously but the code still shows 4.3.10.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.github/workflows/flutter_integration.yaml(3 hunks).tool-versions(1 hunks)android/build.gradle(3 hunks)example/android/app/build.gradle(1 hunks)example/android/settings.gradle(1 hunks)test_integration/android/app/build.gradle(2 hunks)test_integration/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)test_integration/android/settings.gradle(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- example/android/settings.gradle
- example/android/app/build.gradle
- test_integration/android/gradle/wrapper/gradle-wrapper.properties
⏰ 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). (4)
- GitHub Check: android (29)
- GitHub Check: android (24)
- GitHub Check: ios
- GitHub Check: android
🔇 Additional comments (14)
.github/workflows/flutter_integration.yaml (2)
20-20: LGTM: Flutter version updated consistently.The Flutter version update to 3.29 aligns with the version specified in
.tool-versionsand supports the overall Android Gradle upgrade objectives.Also applies to: 56-56
68-68: Good addition: Explicit APK build before testing.Adding
flutter build apkbeforeflutter driveensures the APK is built before the driver tests run, which is a more explicit and reliable approach than relying on implicit build triggers.test_integration/android/app/build.gradle (2)
14-22: LGTM: Java 17 upgrade configured correctly.The Java 17 language level upgrade is properly configured with:
- Core library desugaring enabled to support Java 8+ APIs on older Android versions
- Both
sourceCompatibilityandtargetCompatibilityset to Java 17- Kotlin
jvmTargetaligned to Java 17These changes are consistent with the broader Android toolchain upgrade.
48-50: LGTM: Desugaring dependency added.The
desugar_jdk_libs:2.0.3dependency is correctly added to support thecoreLibraryDesugaringEnabledflag, allowing the use of modern Java APIs on older Android versions.test_integration/android/settings.gradle (1)
22-22: LGTM: Kotlin plugin updated to latest version.The Kotlin Android plugin version 2.2.20 is current and compatible with the AGP 8.x upgrade.
android/build.gradle (8)
2-4: LGTM: SNAPSHOT version with clarifying comments.The version suffix and comments appropriately indicate this is a development artifact not intended for public distribution.
9-9: Good change: Migrated from deprecated jcenter to mavenCentral.JCenter has been deprecated and shut down, so this migration to Maven Central is necessary and correct.
31-31: LGTM: firebase-messaging minor version update.The Firebase Messaging dependency update from 23.0.4 to 23.0.6 is a patch-level update that should be compatible.
38-39: Excellent: Dynamic compileSdkVersion from Flutter.Using
flutter.compileSdkVersionwith a fallback to 34 ensures the plugin automatically uses the Flutter SDK's recommended compile SDK version, addressing one of the PR objectives (issue #577 requested compileSdkVersion 36).
47-49: LGTM: Java 17 language level upgrade.The Java compatibility settings have been correctly updated to Java 17, consistent with modern Android development requirements and the broader toolchain upgrade.
56-56: LGTM: MultiDex enabled for minSdk 19.Enabling MultiDex is necessary when using Java 8+ language APIs via desugaring on Android versions with minSdk < 21. This is correctly configured.
67-71: Excellent addition: Compiler lint warnings enabled.Enabling
-Xlint:uncheckedand-Xlint:deprecationwill surface unchecked type conversions and deprecated API usage at compile time, improving code quality and maintainability.
28-28: Upgrade to ably-android 1.4.1 is safe—no breaking changes identified.The ably-android SDK v1.4.1 contains bug fixes and minor improvements, not breaking changes, making this upgrade from 1.2.40 backward-compatible with no required migration steps.
.tool-versions (1)
1-1: Let me verify CocoaPods 1.15.2 compatibility with Ruby 3.4.5:Ruby 3.4.5 is compatible with the project's CocoaPods 1.15.2 tooling.
CocoaPods works fine with newer Ruby versions like 3.4.x, confirming that the upgrade from Ruby 3.0.1 to 3.4.5 poses no compatibility issues with CocoaPods 1.15.2, which is used in this project's Podfiles.
a4544d0 to
30249e1
Compare
There was a problem hiding this comment.
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 (1)
.github/workflows/flutter_integration.yaml (1)
9-31: Fix undefined matrix.device reference in iOS job.The iOS job (line 16) references
${{ matrix.device }}but no matrix strategy is defined for this job. This will cause the workflow to fail with an undefined variable error. The Android job correctly defines a matrix strategy withapi-level: [24, 29], but the iOS job has none.Replace
${{ matrix.device }}with either:
- A specific device name (e.g.,
'iPhone 14'), or- Add a matrix strategy to the iOS job if multiple device variants are needed
♻️ Duplicate comments (2)
test_integration/android/settings.gradle (2)
21-21: Clarify AGP version alignment across modules.The Android Gradle Plugin version here is 8.7.0, but
android/build.gradledeclares AGP 8.12.1. A past review marked this inconsistency as addressed in commit 500c319, yet the current code still shows 8.7.0. Confirm whether:
- The AGP versions should be aligned to 8.12.1 across both modules, or
- Test integration is intentionally using a different AGP version.
If alignment is required, update to 8.12.1.
21-23: Google Services plugin version 4.3.10 is incompatible with AGP 8.x and will crash on build.The
com.google.gms.google-servicesplugin at version 4.3.10 does not support the AGP 8.x variant APIs and will fail withNoSuchMethodErrorwhen the settings file loads. Update to a 4.4.x release (e.g., 4.4.2).Apply this diff:
- id "com.google.gms.google-services" version "4.3.10" apply false + id "com.google.gms.google-services" version "4.4.2" apply false
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
.github/workflows/flutter_integration.yaml(3 hunks).tool-versions(1 hunks)android/build.gradle(3 hunks)android/src/main/java/io/ably/flutter/plugin/StreamsChannel.java(3 hunks)example/android/app/build.gradle(1 hunks)example/android/settings.gradle(1 hunks)test_integration/android/app/build.gradle(2 hunks)test_integration/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)test_integration/android/settings.gradle(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- test_integration/android/app/build.gradle
- example/android/settings.gradle
- example/android/app/build.gradle
- test_integration/android/gradle/wrapper/gradle-wrapper.properties
⏰ 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: ios
- GitHub Check: android
- GitHub Check: android (29)
- GitHub Check: android (24)
- GitHub Check: ios
🔇 Additional comments (10)
.github/workflows/flutter_integration.yaml (2)
20-20: Flutter version bumps are well-motivated.Upgrading Flutter from 3.24 to 3.29 in both iOS and Android paths aligns with the PR objective to support the newer Flutter SDK alongside the Java 17 and Gradle 8.12.1 upgrades.
Confirm that Flutter 3.29 is compatible with Java 17 and Gradle 8.12.1, particularly for Android build compatibility.
Also applies to: 56-56
68-68: APK build step improves Android test reliability.Adding
flutter build apk &&beforeflutter driveensures the APK is built before running integration tests, which is the correct dependency order for Android emulator-based testing.Also applies to: 68-68
android/src/main/java/io/ably/flutter/plugin/StreamsChannel.java (3)
26-26: LGTM!The import is necessary for the locale-aware formatting changes below.
161-163: LGTM! Locale-independent logging.Using
Locale.ROOTensures consistent log formatting across all locales, which is important for debugging and log analysis.
171-174: LGTM! Critical fix for identifier consistency.Using
Locale.ROOTfor the stream name ensures locale-independent formatting. This is essential since the name is used as a Flutter message channel identifier, and locale-specific formatting could cause channel resolution failures.android/build.gradle (4)
31-31: Firebase Cloud Messaging dependency update looks good.The upgrade from 23.0.4 to 23.0.6 is a patch bump and should maintain API compatibility while bringing bug fixes.
38-48: Dynamic compileSdkVersion fallback and Java 17 compatibility align well with PR objectives.Using
flutter.compileSdkVersion ?: 34allows flexibility with older Flutter SDK versions while maintaining a sensible default, and setting Java 17 compatibility brings the plugin in line with modern AGP and the broader ecosystem.
56-56: Multidex enabled appropriately for Java 17 and AGP 8.x.Enabling
multiDexEnabledis good practice when targeting modern Java and AGP versions, especially with desugaring enabled. No concerns here.
67-71: Compiler warning flags for unchecked and deprecation align with PR objectives.The gradle block correctly injects
-Xlint:uncheckedand-Xlint:deprecationinto all JavaCompile tasks, making deprecated/unsafe code visible during builds per the stated PR goal..tool-versions (1)
1-1: No compatibility issues identified. Ruby 3.4.5 is compatible with project tooling.Verification confirms:
- No Gemfile or managed Ruby dependencies
- Podfiles have no explicit Ruby version constraints
- CocoaPods operations are abstracted through Flutter's tooling, not direct system Ruby usage
- CI workflows don't explicitly manage Ruby versions; they rely on Flutter's abstractions
- No Fastlane, custom build scripts, or Ruby rake files found
The Ruby version update from 3.0.1 (EOL March 2024) to 3.4.5 is safe and compatible with the existing tooling. While unrelated to the Android Gradle/Java updates mentioned in the PR description, this is a beneficial housekeeping change with no breaking compatibility concerns.
| sourceCompatibility = JavaVersion.VERSION_1_8 | ||
| targetCompatibility = JavaVersion.VERSION_1_8 | ||
| sourceCompatibility = JavaVersion.VERSION_17 | ||
| targetCompatibility = JavaVersion.VERSION_17 |
There was a problem hiding this comment.
@coderabbitai This change shouldn't affect older android platforms right
There was a problem hiding this comment.
Pull Request Overview
This PR updates the Android Gradle build configuration to support newer Flutter SDK versions (3.29), addressing compatibility issues. The changes primarily focus on upgrading build tooling versions and modernizing Java/Kotlin compilation targets.
- Upgraded Gradle from 7.5 to 8.10.2/8.12.1 and updated Android Gradle Plugin to 8.7.0/8.12.1
- Migrated Java compilation from version 1.8 to version 17 across all modules
- Updated Kotlin plugin to version 2.2.20 and added compiler warning flags
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test_integration/android/settings.gradle | Updated Android and Kotlin plugin versions, added Google services plugin |
| test_integration/android/gradle/wrapper/gradle-wrapper.properties | Upgraded Gradle wrapper from 7.5 to 8.10.2 |
| test_integration/android/app/build.gradle | Added core library desugaring and updated Java/Kotlin targets to version 17 |
| example/android/settings.gradle | Updated Kotlin plugin to version 2.2.20 |
| example/android/app/build.gradle | Updated Java/Kotlin compilation targets to version 17 |
| android/build.gradle | Updated AGP to 8.12.1, Java targets to 17, Firebase Messaging, and added compiler warnings |
| .tool-versions | Updated Ruby to 3.4.5 and Flutter to 3.29.0 |
| .github/workflows/flutter_integration.yaml | Updated Flutter version to 3.29 and added APK build step before tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
android/build.gradle (2)
13-13: Verify Gradle wrapper version compatibility with AGP 8.12.1.Per the official Android Gradle Plugin compatibility matrix, AGP 8.12.x requires Gradle 8.13 or higher. Ensure that
android/gradle/wrapper/gradle-wrapper.propertieshas been updated to use Gradle 8.13+ in itsdistributionUrlproperty. This file shows only the AGP version; the wrapper version is managed separately. If the wrapper has not been updated, the build will fail due to version incompatibility.Please confirm the Gradle wrapper has been updated by running:
#!/bin/bash cat android/gradle/wrapper/gradle-wrapper.properties | grep distributionUrl
46-49: AddcoreLibraryDesugaringEnabled = truetocompileOptions.The
compileOptionsblock setssourceCompatibilityandtargetCompatibilityto Java 17, and line 34 declares a dependency ondesugar_jdk_libs:1.1.5. WithminSdkVersionat 19, the desugaring library must be explicitly enabled incompileOptions, otherwise Java 17 standard library APIs will not work on older Android devices. The flag is missing and should be added.Apply this diff:
compileOptions { sourceCompatibility = JavaVersion.VERSION_17 targetCompatibility = JavaVersion.VERSION_17 + coreLibraryDesugaringEnabled = true }
🧹 Nitpick comments (1)
android/build.gradle (1)
38-38: Fix comment grammar.The comment should use plural to agree with "Older versions": "don't provide" instead of "doesn't provide".
Apply this diff:
- // Older versions of the Flutter SDK doesn't provide `compileSdkVersion`, so it defaults to 34 + // Older versions of the Flutter SDK don't provide `compileSdkVersion`, so it defaults to 34
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
android/build.gradle(3 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). (5)
- GitHub Check: android
- GitHub Check: ios
- GitHub Check: android (24)
- GitHub Check: ios
- GitHub Check: android (29)
🔇 Additional comments (1)
android/build.gradle (1)
67-71: Compiler flags addition looks good.The
gradle.projectsEvaluatedblock correctly adds unchecked and deprecation warnings to Java compilation, which aligns with the PR objective to add compiler flags and supports better visibility into potential issues when targeting Java 17.
- Upgraded Gradle to 8.12.1 - Changed compile and target compatibility to Java 17 - Set `compileSdkVersion` to use Flutter's value. - Added compiler flags for unchecked and deprecation warnings.
fe2893b to
2a3d17b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.github/workflows/flutter_integration.yaml(2 hunks).tool-versions(1 hunks)android/build.gradle(2 hunks)example/android/app/build.gradle(1 hunks)example/android/settings.gradle(1 hunks)test_integration/android/app/build.gradle(2 hunks)test_integration/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)test_integration/android/settings.gradle(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/workflows/flutter_integration.yaml
- test_integration/android/gradle/wrapper/gradle-wrapper.properties
- example/android/app/build.gradle
- test_integration/android/settings.gradle
- test_integration/android/app/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). (4)
- GitHub Check: android (29)
- GitHub Check: ios
- GitHub Check: android (24)
- GitHub Check: ios
🔇 Additional comments (5)
example/android/settings.gradle (1)
22-22: LGTM! Kotlin plugin upgrade is valid.The upgrade to Kotlin 2.2.20 is appropriate and aligns with the broader Android tooling modernization in this PR. Based on learnings, this version is published and compatible.
android/build.gradle (3)
2-4: LGTM! Clear documentation of snapshot versioning.The added comments clarify that this build.gradle is used by Flutter itself and that artifacts are not intended for external distribution. The snapshot suffix follows Gradle conventions.
31-31: LGTM! Firebase messaging dependency updated.The upgrade from 23.0.4 to 23.0.6 aligns with the PR objective to update outdated Android dependencies. This is a patch version update that likely includes bug fixes and security improvements.
57-61: LGTM! Compiler warnings enabled as intended.The addition of
-Xlint:uncheckedand-Xlint:deprecationflags aligns with the PR objectives and follows best practices for catching potential issues at compile time. The implementation usinggradle.projectsEvaluatedensures these flags are applied after all projects are configured..tool-versions (1)
1-1: I'll verify Ruby 3.4.5 availability and search for other dependency manifests in the codebase.Now let me search for alternative dependency files to confirm whether the project has Ruby dependencies:
Based on my verification:
- Ruby 3.4.5 was released in July 2025
- Since upstream paid great attention to source compatibility, no changes to code are needed for Ruby 3.4
- The script execution found no Gemfile in the repository, meaning this project has no explicit Ruby gem dependencies to verify
The
.tool-versionsfile is used by the asdf version manager to specify which Ruby version to use in development—it is not a dependency manifest. The original review comment assumes the presence of gem dependencies that don't exist in this codebase.No action required—Ruby 3.4.5 is published and available. Since the project contains no Gemfile or gem dependencies, there are no compatibility constraints to verify.
we never used any API that needs desugaring, like streams, `java.time.*`, etc. No need to overcomplicate things for end users.
2a3d17b to
1566840
Compare
Resolves #577, #578
compileSdkVersionto use Flutter's value.Summary by CodeRabbit