Refactor code inside flutter.groovy #160250

AbdeMohlbi · 2024-12-13T17:20:17Z

remove unnecessary spacing
remove the usage of System.env because it is dynamic .
the getProperty have the ability to take a second argument wich will be the default value
android studio linter is prompting Consider using 'configureEach' to avoid unnecessary configuration
android studio linter is prompting Call to 'get' can be keyed access
android studio linter is prompting Cannot determine type of 'o'

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
All existing and new tests are passing.
I followed the breaking change policy and added Data Driven Fixes where supported.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

AbdeMohlbi · 2024-12-13T18:27:27Z

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

-        String result = project.hasProperty(name) ? project.property(name) : null
-        result = result ?: localProperties?.getProperty(name)
-        return result ?: defaultValue
+        return project.findProperty(name) ?: localProperties?.getProperty(name, defaultValue)


findProperty:Returns the value of the given property or null if not found. This method locates a property as follows:

If this project object has a property with the given name, return the value of the property.

If this project has an extension with the given name, return the extension.

If this project's convention object has a property with the given name, return the value of the property.

If this project has an extra property with the given name, return the value of the property.

If this project has a task with the given name, return the task.
Search up through this project's ancestor projects for a convention property or extra property with the given name.

If not found, null value is returned.

potentially breaking because of the exact value this solution brings. Existing conflicting named properties that are in the newly discovered locations could break builds. I am not 100% opposed but it worth looking at.

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

AbdeMohlbi · 2024-12-13T20:53:13Z

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

            project.tasks.register("output${variant.name.capitalize()}AppLinkSettings") {
                description "stores app links settings for the given build variant of this Android project into a json file."
-                variant.outputs.all { output ->
+                variant.outputs.configureEach { output ->


android studio linter is prompting Consider using 'configureEach' to avoid unnecessary configuration

@gmackall I feel like we have had to revert configureEach conversions before but cant recall the circumstances.

@gmackall double check me here.

I also have vaguely have that feeling, but can't remember any specific time. This looks like a good change to me according to https://docs.gradle.org/current/userguide/task_configuration_avoidance.html, and all tests pass, so I'm comfortable landing.

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

flutter-dashboard · 2024-12-13T21:19:15Z

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.

stuartmorgan-g · 2024-12-16T16:30:11Z

test-exempt: code refactor with no semantic change

reidbaker

I am headed out for vacation. @gmackall will be the one to approve this pr before merging.

reidbaker · 2024-12-19T21:45:32Z

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

            project.tasks.register("output${variant.name.capitalize()}AppLinkSettings") {
                description "stores app links settings for the given build variant of this Android project into a json file."
-                variant.outputs.all { output ->
+                variant.outputs.configureEach { output ->


@gmackall I feel like we have had to revert configureEach conversions before but cant recall the circumstances.

reidbaker · 2024-12-19T21:47:43Z

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

-        String result = project.hasProperty(name) ? project.property(name) : null
-        result = result ?: localProperties?.getProperty(name)
-        return result ?: defaultValue
+        return project.findProperty(name) ?: localProperties?.getProperty(name, defaultValue)


potentially breaking because of the exact value this solution brings. Existing conflicting named properties that are in the newly discovered locations could break builds. I am not 100% opposed but it worth looking at.

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

bkonyi · 2025-01-16T21:02:13Z

Friendly bump @flutter/android-reviewers.

reidbaker · 2025-01-17T15:23:11Z

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

            project.tasks.register("output${variant.name.capitalize()}AppLinkSettings") {
                description "stores app links settings for the given build variant of this Android project into a json file."
-                variant.outputs.all { output ->
+                variant.outputs.configureEach { output ->


@gmackall double check me here.

gmackall · 2025-01-28T18:19:12Z

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

            project.tasks.register("output${variant.name.capitalize()}AppLinkSettings") {
                description "stores app links settings for the given build variant of this Android project into a json file."
-                variant.outputs.all { output ->
+                variant.outputs.configureEach { output ->


I also have vaguely have that feeling, but can't remember any specific time. This looks like a good change to me according to https://docs.gradle.org/current/userguide/task_configuration_avoidance.html, and all tests pass, so I'm comfortable landing.

Roll Flutter from c1ffaa9 to b007899 (43 revisions) flutter/flutter@c1ffaa9...b007899 2025-01-29 tessertaha@gmail.com Fix `Tab` linear and elastic animation blink (flutter/flutter#162315) 2025-01-29 pateltirth454@gmail.com Pass-through `textInputAction` in `DropdownMenu` (flutter/flutter#162309) 2025-01-29 38378650+hgraceb@users.noreply.github.com Fix scrollUntilVisible in WidgetTester (flutter/flutter#159582) 2025-01-29 pateltirth454@gmail.com Pass-through `maxLines` in `DropdownMenu` (flutter/flutter#161903) 2025-01-29 gaganyadav80@gmail.com fix: appbar leading width is not square for custom toolbar height (flutter/flutter#161880) 2025-01-29 chinmaygarde@google.com [DisplayList] Don't call Skia Ganesh methods when its not available. (flutter/flutter#162345) 2025-01-29 reidbaker@google.com Update README.md to include googler post verification steps (flutter/flutter#162272) 2025-01-29 kevmoo@users.noreply.github.com [engine, web] return switch expressions in many places (flutter/flutter#162336) 2025-01-29 reidbaker@google.com Update README.md to not have engine link for android (flutter/flutter#162330) 2025-01-29 bkonyi@google.com Reland "[ Widget Previews ] Add support for detecting previews and generating code (#161911)"" (flutter/flutter#162337) 2025-01-29 34871572+gmackall@users.noreply.github.com Add instructions to download the Gradle wrapper to FGP readme, and add to gitignore (flutter/flutter#162332) 2025-01-29 matanlurey@users.noreply.github.com Fix tests to prepare for `--explicit-package-dependencies` and a bug. (flutter/flutter#162289) 2025-01-29 matanlurey@users.noreply.github.com Add a currently unused `runs_in_merge_queue` property to `Linux analyze`. (flutter/flutter#162335) 2025-01-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[ Widget Previews ] Add support for detecting previews and generating code (#161911)" (flutter/flutter#162327) 2025-01-28 58529443+srujzs@users.noreply.github.com Support hot restart for DDC library bundle format (flutter/flutter#162123) 2025-01-28 30870216+gaaclarke@users.noreply.github.com Started adjusting uvs to match pixel snapping. (flutter/flutter#162049) 2025-01-28 mohellebiabdessalem@gmail.com Refactor code inside flutter.groovy (flutter/flutter#160250) 2025-01-28 47866232+chunhtai@users.noreply.github.com Table implements redepth (flutter/flutter#162282) 2025-01-28 bkonyi@google.com [ Widget Previews ] Add support for detecting previews and generating code (flutter/flutter#161911) 2025-01-28 andrewrkolos@gmail.com remove dependency on `Usage` from `Pub` class (flutter/flutter#162279) 2025-01-28 engine-flutter-autoroll@skia.org Roll Packages from 258f6dc to 02c6fef (6 revisions) (flutter/flutter#162313) 2025-01-28 matanlurey@users.noreply.github.com Remove `scenario_app/android` and rename to `ios_scenario_app`. (flutter/flutter#160992) 2025-01-28 matanlurey@users.noreply.github.com Apparently it is illegal to use `stderr` in this project. (flutter/flutter#162294) 2025-01-28 aam@google.com Fix update_engine_version_test in presence of FLUTTER_PREBUILT_ENGINE_VERSION env vars. (flutter/flutter#162270) 2025-01-28 matanlurey@users.noreply.github.com Add missing `properties: ...` and move to presubmit. (flutter/flutter#162170) 2025-01-27 jonahwilliams@google.com [Impeller] make swapchain related external fence/semaphore extensions optional. (flutter/flutter#162205) 2025-01-27 49699333+dependabot[bot]@users.noreply.github.com Bump the all-github-actions group with 2 updates (flutter/flutter#162277) 2025-01-27 chinmaygarde@google.com Don't depend on Dart from FML. (flutter/flutter#162271) 2025-01-27 chinmaygarde@google.com [DisplayList] Move nested canvas enums into their own TU. (flutter/flutter#162037) 2025-01-27 importRyan@gmail.com Avoid iOS text selection crash by returning nil range (flutter/flutter#161996) 2025-01-27 mohellebiabdessalem@gmail.com fix `felt` link to point to flutter repo instead of the engine repo (flutter/flutter#161423) 2025-01-27 matanlurey@users.noreply.github.com Enable the Android Engine OpenGLES/Vulkan suites. (flutter/flutter#162258) 2025-01-27 1961493+harryterkelsen@users.noreply.github.com [canvaskit] Fix debug build for CanvasKit (flutter/flutter#162198) 2025-01-27 engine-flutter-autoroll@skia.org Roll Packages from 3d3ab7b to 258f6dc (19 revisions) (flutter/flutter#162254) 2025-01-25 matanlurey@users.noreply.github.com Pin `customer_testing` to the SHA specified in `tests.version` (flutter/flutter#162048) 2025-01-25 matanlurey@users.noreply.github.com Formalize `update_engine_version.{sh|ps1}`. (flutter/flutter#162118) 2025-01-25 pateltirth454@gmail.com Rename 'SelectionChangedCause.scribble' to 'SelectionChangedCause.stylusHandwriting' (flutter/flutter#161518) 2025-01-25 jacksongardner@google.com Don't install xcode when doing `local_engine` web builds on mac. (flutter/flutter#162164) 2025-01-25 matanlurey@users.noreply.github.com Force Impeller backend for `android_engine_test`, and test both OpenGLES and Vulkan (flutter/flutter#162089) 2025-01-24 jonahwilliams@google.com [Impeller] when a command pool has many unused buffers, reset with release resources flag. (flutter/flutter#162171) 2025-01-24 mdebbar@google.com [web] Remove HTML renderer from framework tests (flutter/flutter#162038) 2025-01-24 jonahwilliams@google.com [Impeller] Skip clip entity replay that cannot impact current clip. (flutter/flutter#162113) 2025-01-24 de.frankenapps@gmail.com Update Android integration test package for newer AGP (flutter/flutter#161856) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: ...

small fixes 1

602d86e

github-actions bot added platform-android Android applications specifically tool Affects the "flutter" command-line tool. See also t: labels. labels Dec 13, 2024

AbdeMohlbi added 3 commits December 13, 2024 18:27

more fixes

4a2ccc0

refactor extensions

7ee7d6e

refactor resolveProperty function

e4d0b57

AbdeMohlbi commented Dec 13, 2024

View reviewed changes

AbdeMohlbi added 6 commits December 13, 2024 20:20

test 1 to add BuildType

bcabb0a

roll back

2513da1

attempt 2

bbe13a2

attempt 2.1

643e5c9

Merge branch 'master' into patch-35

f76f802

roll back

ed0f066

AbdeMohlbi changed the title ~~fix lints inside flutter.groovy~~ refactor code inside flutter.groovy Dec 13, 2024

AbdeMohlbi commented Dec 13, 2024

View reviewed changes

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy Show resolved Hide resolved