Revert "[AGP 9] Bumping KGP error minimum to 2.0.0 (#184385)"#184455
Revert "[AGP 9] Bumping KGP error minimum to 2.0.0 (#184385)"#184455jason-simmons wants to merge 1 commit into
Conversation
|
An existing Git SHA, To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request downgrades the version thresholds for Gradle, Android Gradle Plugin (AGP), and Kotlin Gradle Plugin (KGP) within the dependency checker and associated tests. Review feedback identifies inconsistencies between the updated test constants and the template versions in gradle_utils.dart, a version mismatch between the tool's KGP dependency and its error threshold, and concerns regarding the removal of maintainability comments.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/flutter_tools/gradle/src/test/kotlin/DependencyVersionCheckerTest.kt (56-59)
The updated constants SUPPORTED_GRADLE_VERSION, SUPPORTED_AGP_VERSION, and SUPPORTED_KGP_VERSION are inconsistent with the template versions defined in packages/flutter_tools/lib/src/android/gradle_utils.dart (which are 8.14, 8.11.1, and 2.2.20 respectively). The comments on lines 54-55 explicitly state that these values should match the template. To maintain consistency, either the template versions in gradle_utils.dart should also be reverted, or these constants should be updated to match the current templates.
packages/flutter_tools/gradle/build.gradle.kts (55)
The kotlin-gradle-plugin version is being reverted to 1.8.0. However, DependencyVersionChecker.kt (line 108) defines the error threshold for users at 1.8.10. It is recommended to align the tool's own build dependencies with the minimum supported version to ensure compatibility. Additionally, the cross-reference comment was removed; it should be kept to ensure future updates are synchronized.
// When bumping, also update:
// * KGP error version in packages/flutter_tools/gradle/src/main/kotlin/DependencyVersionChecker.kt
implementation("org.jetbrains.kotlin:kotlin-gradle-plugin:1.8.10")packages/flutter_tools/lib/src/android/gradle_utils.dart (32-34)
The removal of these "When bumping, also update" comments reduces the maintainability of the codebase. These cross-references are essential for ensuring that version updates are synchronized across the tool, the dependency checker, and the test suites. Please consider retaining them to adhere to the principle of optimizing for readability.
References
- Optimize for readability: Code is read more often than it is written. (link)
This reverts commit bb873e8.
05f4425 to
a08777f
Compare
|
I have filed #184456 and am marking those tests as skipped. The failure is that this test needs a higher agp version. |
|
I recommend not landing this revert. |
|
SGTM - closing this. |
|
Thank you for proactively triggering a revert. |
Reverts: #184385
Initiated by: jason-simmons
Reason for reverting: This is causing errors in the
build_android_host_app_with_module_sourceintegration test.Example: https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20build_android_host_app_with_module_source/7845/overview
https://github.com/flutter/flutter/blob/master/dev/integration_tests/pure_android_host_apps/host_app_kotlin_gradle_dsl/gradle/libs.versions.toml needs to be updated.
Original PR Author: jesswrd
Reviewed By: {gmackall}
This change reverts the following previous change:
Bumped KGP error and warn versions to support Built-in Kotlin migration. AGP and Gradle error and warn versions had to be bumped as a result of bumping KGP error and warn versions.
This will be CP'ed to beta and stable.
Fixes #184384
Addresses #181557