[AGP 9] Update AGP Error#185043
Conversation
reidbaker
left a comment
There was a problem hiding this comment.
"Developers should not be running into this error at all due to . " Due to what?
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request replaces the generic AGP 9 warning with specific documentation URLs and updated error messages in gradle_errors.dart, while updating the corresponding test suite. Feedback suggests restoring the file path in the Kotlin migration error message for consistency, correcting grammar and capitalization in the DSL opt-out message, and fixing a spacing mismatch in the test expectations to prevent test failures.
| globals.printBox(''' | ||
| ${globals.logger.terminal.warningMark} Starting AGP 9+, the default has become built-in Kotlin. | ||
| This results in a build failure when applying the kotlin-android plugin. | ||
| To resolve this, migrate to built-in Kotlin. | ||
| \nFor instructions on how to migrate, see: $kMigrateToBuiltInKotlinDocsUrl''', title: _boxTitle); |
There was a problem hiding this comment.
For consistency with other error handlers in this file (like useNewAgpDslErrorHandler), please include the file path where the error occurred and adjust the spacing to group the resolution steps together. This aligns with the principle that error messages should be useful and consistent.
References
- Error messages should be useful: Every error message is an opportunity to make someone love our product. (link)
| \nTo resolve this update flutter or opt out of `android.newDsl`. For instructions on how to opt out, see: https://docs.flutter.dev/release/breaking-changes/migrate-to-agp-9 | ||
| ${globals.logger.terminal.warningMark} Starting AGP 9+, only the new DSL interface will be read. | ||
| This results in a build failure when applying the Flutter Gradle plugin at ${appGradleFile.path}. | ||
| \nTo resolve this update flutter or opt out of `android.newDsl`. |
There was a problem hiding this comment.
Add a comma after 'this' and capitalize 'Flutter' for better readability and correctness. This ensures the error message is professional and clear.
References
- Error messages should be useful: Every error message is an opportunity to make someone love our product. (link)
| contains( | ||
| 'Starting AGP 9+, the default has become built-in Kotlin. This results in a build failure when', | ||
| ), | ||
| contains(' This results in a build failure when applying the kotlin-android plugin'), |
There was a problem hiding this comment.
Code Review
This pull request updates AGP 9 migration error messages in the Flutter tools by replacing a generic warning constant with specific documentation URLs for built-in Kotlin migration and new DSL opt-out. The review feedback suggests restoring the Gradle file path in the Kotlin plugin error message to maintain consistency and provide better context for developers. Additionally, there are suggestions to correct capitalization and punctuation in the DSL error message and to fix a leading space in a test expectation that would otherwise cause a test failure.
| globals.printBox(''' | ||
| ${globals.logger.terminal.warningMark} Starting AGP 9+, the default has become built-in Kotlin. | ||
| This results in a build failure when applying the kotlin-android plugin. | ||
| To resolve this, migrate to built-in Kotlin. | ||
| \nFor instructions on how to migrate, see: $kMigrateToBuiltInKotlinDocsUrl''', title: _boxTitle); |
There was a problem hiding this comment.
The updated error message removes the path to the Gradle file where the failure occurred. Including the file path (appGradleFile.path) is helpful for developers to identify where the change is needed, and it maintains consistency with the error message in useNewAgpDslErrorHandler (line 712).
| globals.printBox(''' | |
| ${globals.logger.terminal.warningMark} Starting AGP 9+, the default has become built-in Kotlin. | |
| This results in a build failure when applying the kotlin-android plugin. | |
| To resolve this, migrate to built-in Kotlin. | |
| \nFor instructions on how to migrate, see: $kMigrateToBuiltInKotlinDocsUrl''', title: _boxTitle); | |
| final File appGradleFile = project.android.appGradleFile; | |
| globals.printBox(''' | |
| ${globals.logger.terminal.warningMark} Starting AGP 9+, the default has become built-in Kotlin. | |
| This results in a build failure when applying the kotlin-android plugin at ${appGradleFile.path}. | |
| To resolve this, migrate to built-in Kotlin. | |
| \nFor instructions on how to migrate, see: $kMigrateToBuiltInKotlinDocsUrl''', title: _boxTitle); |
References
- Error messages should be useful: Every error message is an opportunity to make someone love our product. (link)
| \nTo resolve this update flutter or opt out of `android.newDsl`. For instructions on how to opt out, see: https://docs.flutter.dev/release/breaking-changes/migrate-to-agp-9 | ||
| ${globals.logger.terminal.warningMark} Starting AGP 9+, only the new DSL interface will be read. | ||
| This results in a build failure when applying the Flutter Gradle plugin at ${appGradleFile.path}. | ||
| \nTo resolve this update flutter or opt out of `android.newDsl`. |
There was a problem hiding this comment.
The sentence is missing a comma after "To resolve this" and "Flutter" should be capitalized for consistency with other error messages in the tool.
| \nTo resolve this update flutter or opt out of `android.newDsl`. | |
| \nTo resolve this, update Flutter or opt out of `android.newDsl`. |
References
- Optimize for readability: Code is read more often than it is written. (link)
| contains( | ||
| 'Starting AGP 9+, the default has become built-in Kotlin. This results in a build failure when', | ||
| ), | ||
| contains(' This results in a build failure when applying the kotlin-android plugin'), |
There was a problem hiding this comment.
This expectation contains an unexpected leading space (' This results...'), which is inconsistent with the code on line 688 of gradle_errors.dart and with the other test case on line 1701. This will likely cause the test to fail.
| contains(' This results in a build failure when applying the kotlin-android plugin'), | |
| contains('This results in a build failure when applying the kotlin-android plugin'), |
done added link. |
AGP is ready to use. Developers should not be running into this error at all due to support that has been added for Built-in Kotlin flutter#184836. But in the event that they do run into this error, they will be guided to the right docs. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
AGP is ready to use. Developers should not be running into this error at all due to support that has been added for Built-in Kotlin #184836. But in the event that they do run into this error, they will be guided to the right docs.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.