Repository navigation
Conversation
…all Gradle compatibility error messages - Refactored KGP/Gradle and AGP/KGP compatibility checks in gradle_utils.dart into declarative constant lists. - Implemented bidirectional suggestion helper functions to find compatible ranges for Java, Gradle, AGP, and KGP. - Updated project.dart validation to output clear version suggestions for all compatibility violations. - Refactored warning accumulation to prevent descriptions from being overwritten. - Added comprehensive unit and integration tests covering the compatibility suggestions.
github-actions
Bot
added
tool
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Kotlin Gradle Plugin (KGP), Gradle, and Android Gradle Plugin (AGP) compatibility validation logic in gradle_utils.dart by replacing hardcoded conditional blocks with structured data classes and lists. It also updates project.dart to provide detailed version range suggestions in error messages when incompatibilities are detected, and introduces new unit tests to verify these suggestions. The reviewer feedback suggests utilizing the existing oldestDocumentedKgpCompatabilityVersion constant instead of hardcoding a version string, and simplifying the test setup by removing unused template and package configuration code.
# Conflicts: # packages/flutter_tools/lib/src/android/gradle_utils.dart
|
@mboetger there is one last thing I would like you to do. This code duplicates the intent of the work in https://github.com/flutter/flutter/blob/master/packages/flutter_tools/gradle/src/main/kotlin/DependencyVersionChecker.kt Can you take a pass as figuring out if we can have one source of truth for logic? If that is not possible because of the different languages can you try to make one source of data truth that is read by both pieces of code? Finally if that is not a good solution can you add a test that makes sure the values do not get out of sync? |
reidbaker
left a comment
reidbaker
left a comment
There was a problem hiding this comment.
This code update appears to be an ai generated response to my last comment with no human pass before re-requesting review. @mboetger please review the code on your own before requesting another review from me on this pr. My gut is if you had had reviewed this pr coming from someone else you would have found many of the same suggestions I gave. I ended up stopping part way through the review.
single source of truth.
|
I am concerned about the amount of scope creep occurring due the 'single source of truth'. The more I dig into the failing tests I feel like having a runtime dependency on a JSON file is not the correct way of going as it's breaking a lot of the assumptions of some of those tests relying on the flutter root for the JSON file. I think manual step of generating Kotlin and Dart code from single source of truth would alleviate this concern - and some consideration needs to be taken even with that decision. As such, I'm closing this PR. |
Fixes: #167931
Pre-launch Checklist
///).