-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Chore/gradle remove redundant apply #135348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chore/gradle remove redundant apply #135348
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
///).