-
Couldn't load subscription status.
- Fork 23
Added support for compileSdk 36, latest AGP and Gradle versions #21
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: master
Are you sure you want to change the base?
Conversation
WalkthroughUpgrades Android toolchain, SDK levels, Kotlin, Coroutines, and Gradle across root and example projects; aligns Java/JVM to 21; updates AndroidX dependencies; adjusts example app DSL to static values; refreshes Dart/Flutter constraints and dependencies; modernises test binding/messenger usage; minor whitespace cleanup. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/build.gradle (1)
48-50: AGP 8 uses the ‘lint’ block, not ‘lintOptions’.This will fail on recent AGP. Switch to the new DSL (or remove if no longer needed).
Apply:
- lintOptions { - disable 'InvalidPackage' - } + lint { + disable 'InvalidPackage' + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
example/pubspec.lockis excluded by!**/*.lockpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
android/build.gradle(3 hunks)android/gradle/wrapper/gradle-wrapper.properties(1 hunks)example/android/app/build.gradle(1 hunks)example/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)example/android/settings.gradle(1 hunks)example/pubspec.yaml(1 hunks)lib/files.dart(0 hunks)test/gallery_saver_test.dart(2 hunks)
💤 Files with no reviewable changes (1)
- lib/files.dart
🔇 Additional comments (8)
test/gallery_saver_test.dart (3)
6-8: Correct binding initialisation.Using TestWidgetsFlutterBinding and reusing the messenger is the right pattern.
12-14: Mock handler registration via defaultBinaryMessenger is correct.
26-26: Proper teardown of mock handler.example/pubspec.yaml (1)
6-8: Verify CI dependency resolution — Flutter/Dart unavailable in sandboxexample/pubspec.yaml (lines 6–8) sets sdk: '>=3.0.0 <4.0.0' and flutter: '>=3.0.0'; flutter/dart commands were not found in the verification environment, so confirm that running 'flutter pub get' and your CI resolve the bumped dependencies cleanly.
example/android/gradle/wrapper/gradle-wrapper.properties (1)
5-5: Ensure CI and developer JDKs are Java 21.Gradle wrapper now points to gradle-8.14.3 (example/android/gradle/wrapper/gradle-wrapper.properties — distributionUrl=https://services.gradle.org/distributions/gradle-8.14.3-bin.zip) but java -version reports OpenJDK 17.0.16; update CI and local toolchains to JDK 21 (e.g. actions/setup-java java-version: '21'). .github workflows not found in the repo — verify CI configuration.
android/gradle/wrapper/gradle-wrapper.properties (1)
5-5: Add distributionSha256Sum for Gradle 8.14.3Pin the wrapper distribution SHA‑256 to harden supply‑chain integrity.
distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.3-bin.zip +distributionSha256Sum=bd71102213493060956ec229d946beee57158dbd89d0e62b91bca0fa2c5f3531android/build.gradle (1)
5-6: Action: verify forward‑leaning Android/Kotlin/AGP versions
- File: android/build.gradle (lines 5–6; also applies to 13 and 55–60): ext.kotlin_version = '2.2.20', ext.kotlinCoroutinesVersion = '1.10.2' — kotlinx‑coroutines 1.10.2 was published as a companion for Kotlin 2.1.0 and is not explicitly targeted at Kotlin 2.2.20. Align Kotlin ↔ coroutines (downgrade Kotlin or pick a coroutines release that lists Kotlin 2.2.x), or validate with full Android CI/integration tests.
- androidx.core:core-ktx:1.17.0 — artifact exists (pre‑release channels); recent core releases typically require compileSdk ≥ 34. Confirm compileSdk and prefer a stable core-ktx if you need guaranteed support.
- AGP 8.13.0 + Gradle 8.14.3 — AGP↔Gradle pairings are strict; use the Gradle version recommended by the AGP 8.13 release notes rather than an unverified Gradle 8.14.3 build.
Verify and pin compatible versions, then re-run Android build CI and smoke tests.
example/android/settings.gradle (1)
21-22: Version combo sanity check (AGP 8.13.0 + Kotlin 2.2.20)Kotlin 2.2.20 and Gradle 8.14.3 are released and compatible with Java 21; AGP 8.13 is referenced in Android docs but an authoritative AGP 8.13 release-notes page specifying its minimum Gradle version was not located — verify AGP 8.13 is officially released and supports Gradle 8.14.3 / JDK 21.
File: example/android/settings.gradle Lines: 21-22
id "com.android.application" version "8.13.0" apply false id "org.jetbrains.kotlin.android" version "2.2.20" apply false
| android { | ||
| namespace "carnegietechnologies.gallery_saver" | ||
| compileSdkVersion 31 | ||
| compileSdkVersion 36 |
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.
🧹 Nitpick
Optional: modernise to compileSdk (property) for consistency.
- compileSdkVersion 36
+ compileSdk 36📝 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.
| compileSdkVersion 36 | |
| compileSdk 36 |
🤖 Prompt for AI Agents
In android/build.gradle at line 30, replace the legacy compileSdkVersion 36
usage with the modern property form by changing it to compileSdk = 36; update
that single line to use the compileSdk property so it matches current
Gradle/Android DSL conventions and keeps consistency across the project.
| jvmTarget = JavaVersion.VERSION_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.
🧹 Nitpick
Optional: set jvmTarget as a string to avoid Groovy coercion surprises.
- jvmTarget = JavaVersion.VERSION_21
+ jvmTarget = "21"📝 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.
| jvmTarget = JavaVersion.VERSION_21 | |
| } | |
| jvmTarget = "21" | |
| } |
🤖 Prompt for AI Agents
In android/build.gradle around lines 38-39, the jvmTarget is assigned
JavaVersion.VERSION_21 which can lead to Groovy coercion surprises; change the
assignment to use a plain string (e.g., "21") instead of the JavaVersion
constant so the Gradle/Kotlin compiler receives a simple string value for
jvmTarget.
| } | ||
| defaultConfig { | ||
| minSdkVersion 21 | ||
| minSdkVersion 24 |
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.
🧩 Analysis chain
Min SDK raised to 24 — update plugin metadata too.
Ensure the Flutter plugin’s pubspec under flutter.plugin.platforms.android.minSdkVersion is also set to 24, or consumers may get mismatched expectations.
🏁 Script executed:
#!/bin/bash
# Locate plugin pubspec and confirm minSdkVersion advertised to users
fd -a pubspec.yaml
rg -nC2 'flutter:\s*plugin:|platforms:\s*android:|minSdkVersion' -n pubspec.yaml **/pubspec.yamlLength of output: 71
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
/home/jailuser/git/example/pubspec.yaml
/home/jailuser/git/pubspec.yaml
)
for f in "${files[@]}"; do
if [ -f "$f" ]; then
echo "=== $f ==="
sed -n '1,240p' "$f" || true
echo "---- rg matches ----"
rg -nC2 'flutter:\s*plugin' "$f" || true
rg -nC2 'platforms:\s*android' "$f" || true
rg -nC2 'minSdkVersion' "$f" || true
else
echo "MISSING: $f"
fi
doneLength of output: 1248
Min SDK raised to 24 — update plugin pubspec
android/build.gradle sets minSdkVersion to 24; add flutter.plugin.platforms.android.minSdkVersion: 24 to pubspec.yaml (repo root) so the plugin metadata matches. For example, under flutter.plugin.platforms.android add:
flutter:
plugin:
platforms:
android:
package: carnegietechnologies.gallery_saver
pluginClass: GallerySaverPlugin
minSdkVersion: 24
🤖 Prompt for AI Agents
In android/build.gradle around line 45, minSdkVersion was raised to 24; update
the plugin metadata in the repo root pubspec.yaml so they match by adding the
android minSdkVersion under the flutter.plugin.platforms.android block — i.e.,
under flutter: plugin: platforms: android: add the minSdkVersion key with value
24 alongside package and pluginClass so the plugin metadata reflects the new
minimum SDK.
| } | ||
|
|
||
| dependencies { | ||
| implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version" |
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.
Stop using deprecated kotlin-stdlib-jdk7 with Kotlin 2.x.
Use kotlin-stdlib (the jdk7 artifact has been folded/deprecated for a long time).
- implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"
+ implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"📝 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.
| implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version" | |
| implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version" |
🤖 Prompt for AI Agents
In android/build.gradle around line 54, the build uses the deprecated artifact
kotlin-stdlib-jdk7; replace that dependency with the canonical kotlin-stdlib
(keeping the existing $kotlin_version variable) so the implementation line uses
"org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version" instead of the jdk7
artifact to avoid deprecated API usage with Kotlin 2.x.
| implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version" | ||
| api 'androidx.core:core-ktx:1.5.0' | ||
| api 'androidx.core:core-ktx:1.17.0' | ||
| api 'androidx.legacy:legacy-support-v4:1.0.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.
🧹 Nitpick
🧩 Analysis chain
legacy-support-v4 is obsolete on minSdk 24; avoid leaking via API.
If not strictly needed, remove it. If needed but not part of your public API, use implementation. Verify usage before changing.
Potential change:
- api 'androidx.legacy:legacy-support-v4:1.0.0'
+ // TODO: remove if unused; otherwise prefer implementation to avoid leaking it
+ implementation 'androidx.legacy:legacy-support-v4:1.0.0'🏁 Script executed:
#!/bin/bash
# Find usages and API leaks of legacy support types
rg -n -C2 'androidx\.legacy' android | sed -n '1,200p'
# Check if any public API references legacy packages (Kotlin/Java)
rg -nP -C2 'public\s+.*(androidx\.legacy\.)' android || trueLength of output: 442
Replace api with implementation for androidx.legacy:legacy-support-v4
Only occurrence is the declaration in android/build.gradle:56 and no public API references were found — switch to implementation to avoid leaking the legacy library.
- api 'androidx.legacy:legacy-support-v4:1.0.0'
+ // TODO: remove if unused; otherwise prefer implementation to avoid leaking it
+ implementation 'androidx.legacy:legacy-support-v4:1.0.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.
| api 'androidx.legacy:legacy-support-v4:1.0.0' | |
| // TODO: remove if unused; otherwise prefer implementation to avoid leaking it | |
| implementation 'androidx.legacy:legacy-support-v4:1.0.0' |
🤖 Prompt for AI Agents
In android/build.gradle around line 56, the dependency is declared with api
'androidx.legacy:legacy-support-v4:1.0.0' which can leak the library into
consumers; change the configuration from api to implementation so the legacy
support library is internal to the module and not exposed as a transitive public
API.
| android { | ||
| namespace = "com.example.example" | ||
| compileSdk = flutter.compileSdkVersion | ||
| compileSdk = 36 |
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.
🧹 Nitpick
compileSdk 36 — good. Verify NDK and CI JDK toolchains.
No action needed beyond ensuring CI images include Android API 36 and JDK 21.
🤖 Prompt for AI Agents
In example/android/app/build.gradle around line 10, compileSdk is set to 36; no
code changes required in this file, but verify and update CI/NDK toolchains:
ensure CI build images and any Android NDK/toolchain configs include Android API
36 and that CI uses JDK 21 (or an image with JDK 21 installed); if missing,
update CI pipeline images or toolchain installation steps to install Android SDK
platform 36 and JDK 21 so builds match the compileSdk.
| sourceCompatibility JavaVersion.VERSION_21 | ||
| targetCompatibility JavaVersion.VERSION_21 | ||
| } | ||
|
|
||
| kotlinOptions { | ||
| jvmTarget = JavaVersion.VERSION_1_8 | ||
| jvmTarget = JavaVersion.VERSION_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.
🧹 Nitpick
Optional: use string for jvmTarget.
- jvmTarget = JavaVersion.VERSION_21
+ jvmTarget = "21"📝 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.
| sourceCompatibility JavaVersion.VERSION_21 | |
| targetCompatibility JavaVersion.VERSION_21 | |
| } | |
| kotlinOptions { | |
| jvmTarget = JavaVersion.VERSION_1_8 | |
| jvmTarget = JavaVersion.VERSION_21 | |
| } | |
| sourceCompatibility JavaVersion.VERSION_21 | |
| targetCompatibility JavaVersion.VERSION_21 | |
| } | |
| kotlinOptions { | |
| jvmTarget = "21" | |
| } |
| minSdkVersion 24 | ||
| targetSdkVersion 36 | ||
| versionCode = flutter.versionCode |
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.
🧹 Nitpick
Optional: use modern DSL for SDK levels.
Groovy supports both, but aligning with property names improves readability.
- minSdkVersion 24
- targetSdkVersion 36
+ minSdk 24
+ targetSdk 36📝 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.
| minSdkVersion 24 | |
| targetSdkVersion 36 | |
| versionCode = flutter.versionCode | |
| minSdk 24 | |
| targetSdk 36 | |
| versionCode = flutter.versionCode |
🤖 Prompt for AI Agents
In example/android/app/build.gradle around lines 27 to 29, the build.gradle uses
the older property names minSdkVersion and targetSdkVersion; update them to the
modern DSL property names by replacing minSdkVersion with minSdk and
targetSdkVersion with targetSdk while leaving versionCode = flutter.versionCode
unchanged so the file uses the more readable, current property naming.
This PR fixes #20
Summary by CodeRabbit
New Requirements
Chores
Examples
Tests
Style