Updated applying gradle plugin for flutter_plugin_android_lifecycles #7786
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). 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. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
jesswrd
changed the title
jesswrd
changed the title
There was a problem hiding this comment.
I think this apply line is still required
There was a problem hiding this comment.
Yeah, I agree with Reid. Source here (I didn't know about it myself).
Fortunately, it seems like that plugin can also be applied in a declarative way:
Looking at the plugin's source code, it should be possible to apply it in both build.gradle (Plugin<Project>) and settings.gradle (Plugin<Settings>), because the plugin class extends Plugin<Object>.
There was a problem hiding this comment.
jesswrd
force-pushed
the
update_plugin_exapps
branch
2 times, most recently
from
3820166 to
57b37ed
Compare
reidbaker
left a comment
reidbaker
left a comment
There was a problem hiding this comment.
Please wait to merge after approval from @bartekpacia
There was a problem hiding this comment.
overall this looks good, pretty typical migration - apart from the com.google.cloud.artifactregistry.gradle-plugin. I'd say that if you don't find a way to make this specific plugin work when applied through the plugins {} block within some reasonable time, just keep the old way.
Then a separate issue could be created to track migrating only this specific plugin to declarative. Of course it's all "IMHO" and I defer to @reidbaker :D
# Conflicts: # packages/flutter_plugin_android_lifecycle/example/android/build.gradle # packages/flutter_plugin_android_lifecycle/example/android/gradle/wrapper/gradle-wrapper.properties
jesswrd
force-pushed
the
update_plugin_exapps
branch
from
6d40c35 to
5ae23df
Compare
|
Hey @jesswrd, I tried applying the artifact-plugin declaratively on my branch in #7811 I think I succeeded. Feel free to copy stuff from there into this PR. I hope this helps. The only failing test is |
|
@jesswrd before landing this pr Please update the title (at the minimum review WIP) |
Yeah I wrote a test to ensure all packages had the artifact strings. It is ok to update that regex to match or to add a new regex and allow either to pass. |
jesswrd
changed the title
Oops I merged this PR before I saw these comments. I'm gonna apply those changes on the next plugin then circle back. |
flutter/packages@8fbf4cd...9d00fb1 2024-10-09 [email protected] [interactive_media_ads] Adds internal wrapper for Android native `CompanionAd` (flutter/packages#7823) 2024-10-08 [email protected] Disable `SurfaceProducer.Callback` when the surface is disposed manually. (flutter/packages#7827) 2024-10-08 [email protected] Dispose the `ExoPlayer` before `SurfaceProducer`. (flutter/packages#7824) 2024-10-08 [email protected] [pigeon] Use non-nullable generics in example app (flutter/packages#7817) 2024-10-08 [email protected] Manually Roll Flutter (stable) from 4cf269e to 2663184 (5 revisions) (flutter/packages#7819) 2024-10-08 [email protected] [camera] Update iOS Pigeon for non-nullable generics (flutter/packages#7787) 2024-10-08 [email protected] [in_app_purchase] Update Android Pigeon for non-nullable generics (flutter/packages#7788) 2024-10-08 [email protected] [WIP] Updated applying gradle plugin for flutter_plugin_android_lifecycles (flutter/packages#7786) 2024-10-08 [email protected] Manual roll Flutter from ec2e12b to 0917e9d (29 revisions) (flutter/packages#7816) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
3 participants
Updated applying gradle plugins to usage of declarative blocks. Intending on updating all android example apps under packages. Did one as a proof of concept before doing more.
Fixes partially #152656
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.