Chore/gradle remove redundant apply#135348
Chore/gradle remove redundant apply#135348auto-submit[bot] merged 4 commits intoflutter:masterfrom bartekpacia:chore/gradle_remove_redundant_apply
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
gmackall
left a comment
There was a problem hiding this comment.
LGTM, thanks for the cleanup. Will need a test exemption
| id = "dev.flutter.flutter-gradle-plugin" | ||
| implementationClass = "FlutterPlugin" | ||
| } | ||
| // The "flutterAppPluginLoaderPlugin" name isn't used anywhere. |
There was a problem hiding this comment.
what does this comment mean in practice?
(isn't it used on the very next line, for example?)
There was a problem hiding this comment.
It means that flutterAppPluginLoaderPlugin can be changed to some other string (for whatever reason) and it won't affect anything.
Nothing in the codebase references flutterAppPluginLoaderPlugin.
|
test-exempt code removal |
|
auto label is removed for flutter/flutter/135348, due to - The status or check suite Linux android views has failed. Please fix the issues identified (or deflake) before re-applying this label. |
I'm removing an unneded block of configuration from the
settings.gradletemplate. It was introduced by me in #123511. At that time, I did not know that it's unnecessary, and did not test removing it – sorry about that.I learned that it's unnecessary recently, when I asked a question on StackOverflow. More context there.
Pre-launch Checklist
///).