More FlutterPlugin static method conversion#165506
More FlutterPlugin static method conversion#165506auto-submit[bot] merged 27 commits intoflutter:masterfrom
FlutterPlugin static method conversion#165506Conversation
...ges/flutter_tools/test/android_preview_integration.shard/flutter_build_preview_sdk_test.dart
Show resolved
Hide resolved
reidbaker
left a comment
There was a problem hiding this comment.
Spent most of my time reviewing the code change and skimmed the tests. Looks good!
At a high level add ktdocs for any internal but public methods.
|
|
||
| @JvmStatic | ||
| @JvmName("readPropertiesIfExist") | ||
| internal fun readPropertiesIfExist(propertiesFile: File): Properties { |
There was a problem hiding this comment.
Please add a ktdoc to this method also consider a different name. I know the name is the same as before but this sounds like a method that will read something related to project.hasProperty.
| engineVersion: String | ||
| ) { | ||
| val flutterBuildMode: String = buildModeFor(buildType) | ||
| // In AGP 3.5, the embedding must be added as an API implementation, |
There was a problem hiding this comment.
Minimum agp is higher than 3.5 we can remove this branch.
There was a problem hiding this comment.
My impression is that previous comments along the lines of "only used for AGP " or "remove after AGP " haven't been trustworthy. I'd prefer to attempt this removal after the conversion is done. Added a todo comment.
There was a problem hiding this comment.
Yes and removal of the branch would be a code change. A todo makes sense to me.
packages/flutter_tools/gradle/src/test/kotlin/FlutterPluginUtilsTest.kt
Outdated
Show resolved
Hide resolved
packages/flutter_tools/gradle/src/test/kotlin/FlutterPluginUtilsTest.kt
Outdated
Show resolved
Hide resolved
Moves a lot more functionality from `flutter.groovy` to `FlutterPluginUtils.kt` that could be made static. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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]. - [ ] 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. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [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 --------- Co-authored-by: Gray Mackall <[email protected]>
Moves a lot more functionality from `flutter.groovy` to `FlutterPluginUtils.kt` that could be made static. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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]. - [ ] 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. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [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 --------- Co-authored-by: Gray Mackall <[email protected]>
Moves a lot more functionality from
flutter.groovytoFlutterPluginUtils.ktthat could be made static.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.