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 [email protected] Fix `Tab` linear and elastic animation blink (flutter/flutter#162315) 2025-01-29 [email protected] Pass-through `textInputAction` in `DropdownMenu` (flutter/flutter#162309) 2025-01-29 [email protected] Fix scrollUntilVisible in WidgetTester (flutter/flutter#159582) 2025-01-29 [email protected] Pass-through `maxLines` in `DropdownMenu` (flutter/flutter#161903) 2025-01-29 [email protected] fix: appbar leading width is not square for custom toolbar height (flutter/flutter#161880) 2025-01-29 [email protected] [DisplayList] Don't call Skia Ganesh methods when its not available. (flutter/flutter#162345) 2025-01-29 [email protected] Update README.md to include googler post verification steps (flutter/flutter#162272) 2025-01-29 [email protected] [engine, web] return switch expressions in many places (flutter/flutter#162336) 2025-01-29 [email protected] Update README.md to not have engine link for android (flutter/flutter#162330) 2025-01-29 [email protected] Reland "[ Widget Previews ] Add support for detecting previews and generating code (#161911)"" (flutter/flutter#162337) 2025-01-29 [email protected] Add instructions to download the Gradle wrapper to FGP readme, and add to gitignore (flutter/flutter#162332) 2025-01-29 [email protected] Fix tests to prepare for `--explicit-package-dependencies` and a bug. (flutter/flutter#162289) 2025-01-29 [email protected] 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 [email protected] Support hot restart for DDC library bundle format (flutter/flutter#162123) 2025-01-28 [email protected] Started adjusting uvs to match pixel snapping. (flutter/flutter#162049) 2025-01-28 [email protected] Refactor code inside flutter.groovy (flutter/flutter#160250) 2025-01-28 [email protected] Table implements redepth (flutter/flutter#162282) 2025-01-28 [email protected] [ Widget Previews ] Add support for detecting previews and generating code (flutter/flutter#161911) 2025-01-28 [email protected] remove dependency on `Usage` from `Pub` class (flutter/flutter#162279) 2025-01-28 [email protected] Roll Packages from 258f6dc to 02c6fef (6 revisions) (flutter/flutter#162313) 2025-01-28 [email protected] Remove `scenario_app/android` and rename to `ios_scenario_app`. (flutter/flutter#160992) 2025-01-28 [email protected] Apparently it is illegal to use `stderr` in this project. (flutter/flutter#162294) 2025-01-28 [email protected] Fix update_engine_version_test in presence of FLUTTER_PREBUILT_ENGINE_VERSION env vars. (flutter/flutter#162270) 2025-01-28 [email protected] Add missing `properties: ...` and move to presubmit. (flutter/flutter#162170) 2025-01-27 [email protected] [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 [email protected] Don't depend on Dart from FML. (flutter/flutter#162271) 2025-01-27 [email protected] [DisplayList] Move nested canvas enums into their own TU. (flutter/flutter#162037) 2025-01-27 [email protected] Avoid iOS text selection crash by returning nil range (flutter/flutter#161996) 2025-01-27 [email protected] fix `felt` link to point to flutter repo instead of the engine repo (flutter/flutter#161423) 2025-01-27 [email protected] Enable the Android Engine OpenGLES/Vulkan suites. (flutter/flutter#162258) 2025-01-27 [email protected] [canvaskit] Fix debug build for CanvasKit (flutter/flutter#162198) 2025-01-27 [email protected] Roll Packages from 3d3ab7b to 258f6dc (19 revisions) (flutter/flutter#162254) 2025-01-25 [email protected] Pin `customer_testing` to the SHA specified in `tests.version` (flutter/flutter#162048) 2025-01-25 [email protected] Formalize `update_engine_version.{sh|ps1}`. (flutter/flutter#162118) 2025-01-25 [email protected] Rename 'SelectionChangedCause.scribble' to 'SelectionChangedCause.stylusHandwriting' (flutter/flutter#161518) 2025-01-25 [email protected] Don't install xcode when doing `local_engine` web builds on mac. (flutter/flutter#162164) 2025-01-25 [email protected] Force Impeller backend for `android_engine_test`, and test both OpenGLES and Vulkan (flutter/flutter#162089) 2025-01-24 [email protected] [Impeller] when a command pool has many unused buffers, reset with release resources flag. (flutter/flutter#162171) 2025-01-24 [email protected] [web] Remove HTML renderer from framework tests (flutter/flutter#162038) 2025-01-24 [email protected] [Impeller] Skip clip entity replay that cannot impact current clip. (flutter/flutter#162113) 2025-01-24 [email protected] 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