`CommandResultEvent` migrated by eliasyishak · Pull Request #138165 · flutter/flutter

eliasyishak · 2023-11-09T18:34:26Z

Related to tracker issue:

☂️ package:unified_analytics implementation in flutter tool #128251

This event was only called from one file (flutter_command.dart). With the previous implementation, we actually sent 2 events, one for the result of the commandPath and another containing the maxRss value from ProcessInfo.

I have consolidated this down to just one event and used a function to safely get the maxRss value, or return null when if there was an error getting that integer value

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.
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.
All existing and new tests are passing.

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

packages/flutter_tools/lib/src/runner/flutter_command.dart

packages/flutter_tools/lib/src/reporting/unified_analytics.dart

eliasyishak · 2023-11-09T19:51:48Z

packages/flutter_tools/lib/src/runner/flutter_command.dart

    FlutterCommandResult commandResult,
    DateTime startTime,
    DateTime endTime,
+    Analytics analytics,
+    ProcessInfo processInfo,
  ) {


@christopherfujino, is it enough to inject it at this level? Otherwise, I would need to inject these 2 parameters in each of the 70+ FlutterCommand implementations.

or I could possibly create a getter in the abstract class FlutterCommand to fetch these two classes from globals so that each implementation of FlutterCommand has access to it

abstract class FlutterCommand extends Command<void> { Analytics get analytics => globals.analytics; ProcessInfo get processInfo => globals.processInfo; //..... }

you know what, I like that approach (creating getters on FlutterCommand). Then later on, we can set up a method to lazily initialize them.

Cool, that will also make it easy for any other command to directly inject the analytics instance too

And for posterity, when I say "a method to lazily initialize them", I mean a process, not literally a class method.

christopherfujino

LGTM

Roll Flutter from f662150 to e8c2bb1 (66 revisions) flutter/flutter@f662150...e8c2bb1 2023-11-14 [email protected] Reland "Update `framework_test.dart` to remove `ButtonBar` usage and remove references from other clases (#137550) (flutter/flutter#137753) 2023-11-14 [email protected] Consume flutter.js from the engine artifacts. (flutter/flutter#137113) 2023-11-14 [email protected] Roll Flutter Engine from 995ebcefb0d4 to 1b3fd80812c3 (1 revision) (flutter/flutter#138415) 2023-11-14 [email protected] Roll Packages from 17bd92e to 428ba3e (2 revisions) (flutter/flutter#138412) 2023-11-14 [email protected] Roll Flutter Engine from 712094481217 to 995ebcefb0d4 (1 revision) (flutter/flutter#138409) 2023-11-14 [email protected] Roll Flutter Engine from 2c54b5f70c94 to 712094481217 (1 revision) (flutter/flutter#138401) 2023-11-14 [email protected] Roll Flutter Engine from d100e8912eb0 to 2c54b5f70c94 (4 revisions) (flutter/flutter#138399) 2023-11-14 [email protected] Roll Flutter Engine from 74a9de45f128 to d100e8912eb0 (3 revisions) (flutter/flutter#138383) 2023-11-14 [email protected] Roll Flutter Engine from 77b952f3add4 to 74a9de45f128 (3 revisions) (flutter/flutter#138382) 2023-11-14 [email protected] Fixing typo (flutter/flutter#138253) 2023-11-14 [email protected] Roll Flutter Engine from 046ec85dffc6 to 77b952f3add4 (1 revision) (flutter/flutter#138377) 2023-11-13 [email protected] Roll Flutter Engine from db6da000a17e to 046ec85dffc6 (5 revisions) (flutter/flutter#138375) 2023-11-13 [email protected] Finally remove analysis_options_user.yaml (flutter/flutter#138261) 2023-11-13 [email protected] Add a DevTools section to CONTRIBUTING.md (flutter/flutter#137193) 2023-11-13 [email protected] Update DraggableScrollableSheet docs to reflect API change (flutter/flutter#136471) 2023-11-13 [email protected] Clean up synonyms, key code generation. (flutter/flutter#138192) 2023-11-13 [email protected] Roll Flutter Engine from 74ba6c17a488 to db6da000a17e (2 revisions) (flutter/flutter#138364) 2023-11-13 [email protected] Roll Flutter Engine from 5d62f1a2392a to 74ba6c17a488 (1 revision) (flutter/flutter#138362) 2023-11-13 [email protected] Roll Flutter Engine from 7de793d2bb68 to 5d62f1a2392a (1 revision) (flutter/flutter#138358) 2023-11-13 [email protected] Upgrade leak tracker. (flutter/flutter#138283) 2023-11-13 [email protected] Roll Flutter Engine from fe11f3a46bac to 7de793d2bb68 (2 revisions) (flutter/flutter#138353) 2023-11-13 [email protected] Roll Packages from a682189 to 17bd92e (2 revisions) (flutter/flutter#138347) 2023-11-13 [email protected] Roll Flutter Engine from a18ee3c7f57a to fe11f3a46bac (2 revisions) (flutter/flutter#138344) 2023-11-12 [email protected] Roll Flutter Engine from 828e4dbf6693 to a18ee3c7f57a (7 revisions) (flutter/flutter#138332) 2023-11-11 [email protected] Roll Flutter Engine from e2e07eab35ec to 828e4dbf6693 (1 revision) (flutter/flutter#138282) 2023-11-11 [email protected] Roll Flutter Engine from 1c29ce15c528 to e2e07eab35ec (1 revision) (flutter/flutter#138280) 2023-11-11 [email protected] Roll Flutter Engine from aa6753fdbb51 to 1c29ce15c528 (1 revision) (flutter/flutter#138277) 2023-11-11 [email protected] Roll Flutter Engine from 00db306f6f7b to aa6753fdbb51 (1 revision) (flutter/flutter#138269) 2023-11-11 [email protected] Roll Flutter Engine from 9d8a1125640d to 00db306f6f7b (7 revisions) (flutter/flutter#138266) 2023-11-10 [email protected] Only run tests on macOS 12 (flutter/flutter#138260) 2023-11-10 [email protected] Fixes vscode path installed via snap (flutter/flutter#136997) 2023-11-10 [email protected] Docs typo: comprised -> composed (flutter/flutter#137896) 2023-11-10 [email protected] Deprecates onWillAccept and onAccept callbacks in DragTarget. (flutter/flutter#133691) 2023-11-10 [email protected] [macOS] Suppress Xcode 15 createItemModels warning (flutter/flutter#138243) 2023-11-10 [email protected] Roll Flutter Engine from 275ddb296ec9 to 9d8a1125640d (1 revision) (flutter/flutter#138252) 2023-11-10 [email protected] GestureRecognizer should dispatch creation and disposal events. (flutter/flutter#138223) 2023-11-10 [email protected] Roll Flutter Engine from 5c2e16c5a95a to 275ddb296ec9 (1 revision) (flutter/flutter#138249) 2023-11-10 [email protected] Roll Packages from b69f54e to a682189 (3 revisions) (flutter/flutter#138239) 2023-11-10 [email protected] Roll Flutter Engine from e5b75177ac8e to 5c2e16c5a95a (1 revision) (flutter/flutter#138247) 2023-11-10 [email protected] `CommandResultEvent` migrated (flutter/flutter#138165) 2023-11-10 [email protected] Fix #128925 by properly setting the Android Event Source (flutter/flutter#138241) 2023-11-10 [email protected] Roll Flutter Engine from 77349dc8e27b to e5b75177ac8e (2 revisions) (flutter/flutter#138244) 2023-11-10 [email protected] Roll Flutter Engine from a5eab0d281fe to 77349dc8e27b (4 revisions) (flutter/flutter#138237) 2023-11-10 [email protected] Update analytics constructor to include `FLUTTER_HOST` (flutter/flutter#138107) 2023-11-10 [email protected] Prepare the analyze_once test for removal of analysis_options_user support (flutter/flutter#138229) 2023-11-10 [email protected] Roll Flutter Engine from b020893cba29 to a5eab0d281fe (5 revisions) (flutter/flutter#138231) ...

eliasyishak added 2 commits November 9, 2023 13:20

Migration for flutter_command.dart complete

3dd0a75

Update pubspec.yaml

2988cd9

eliasyishak requested a review from christopherfujino as a code owner November 9, 2023 18:34

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Nov 9, 2023

christopherfujino reviewed Nov 9, 2023

View reviewed changes

packages/flutter_tools/lib/src/runner/flutter_command.dart Outdated Show resolved Hide resolved

christopherfujino reviewed Nov 9, 2023

View reviewed changes

packages/flutter_tools/lib/src/reporting/unified_analytics.dart Show resolved Hide resolved

Fixes from code review

a982548

eliasyishak commented Nov 9, 2023

View reviewed changes

eliasyishak mentioned this pull request Nov 9, 2023

☂️ package:unified_analytics implementation in flutter tool #128251

Closed

32 tasks

eliasyishak added 2 commits November 9, 2023 17:09

Use only one getter for ProcessInfo

58ce642

Merge branch 'master' into ga4-migrate-command-result

581d9a3

christopherfujino approved these changes Nov 10, 2023

View reviewed changes

eliasyishak added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 10, 2023

auto-submit bot merged commit 7a278ae into flutter:master Nov 10, 2023

eliasyishak deleted the ga4-migrate-command-result branch November 10, 2023 20:22

engine-flutter-autoroll mentioned this pull request Nov 11, 2023

Roll Flutter from f66215027e14 to e33d4b86270e (42 revisions) flutter/packages#5371

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 11, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

1131c8b

engine-flutter-autoroll mentioned this pull request Nov 11, 2023

Roll Flutter from f66215027e14 to e33d4b86270e (42 revisions) flutter/packages#5372

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 11, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

302b974

engine-flutter-autoroll mentioned this pull request Nov 12, 2023

Roll Flutter from f66215027e14 to e33d4b86270e (42 revisions) flutter/packages#5373

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

daaedec

engine-flutter-autoroll mentioned this pull request Nov 13, 2023

Roll Flutter from f66215027e14 to 4e77e7b10c4a (44 revisions) flutter/packages#5391

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

f33141c

engine-flutter-autoroll mentioned this pull request Nov 13, 2023

Roll Flutter from f66215027e14 to f70a01c69165 (45 revisions) flutter/packages#5392

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

3e2b039

engine-flutter-autoroll mentioned this pull request Nov 13, 2023

Roll Flutter from f66215027e14 to 749daa1af755 (46 revisions) flutter/packages#5393

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

10f8e89

engine-flutter-autoroll mentioned this pull request Nov 13, 2023

Roll Flutter from f66215027e14 to a681ff618ccd (48 revisions) flutter/packages#5394

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

8ac2107

engine-flutter-autoroll mentioned this pull request Nov 14, 2023

Roll Flutter from f66215027e14 to b356bb387fc3 (62 revisions) flutter/packages#5395

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

c8282f0

engine-flutter-autoroll mentioned this pull request Nov 14, 2023

Roll Flutter from f66215027e14 to ff72804498e3 (63 revisions) flutter/packages#5396

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

8c1dffe

engine-flutter-autoroll mentioned this pull request Nov 14, 2023

Roll Flutter from f66215027e14 to 463c02daafa5 (64 revisions) flutter/packages#5397

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

039ce21

engine-flutter-autoroll mentioned this pull request Nov 14, 2023

Roll Flutter from f66215027e14 to 07d73630fbdb (65 revisions) flutter/packages#5398

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

84c244d

engine-flutter-autoroll mentioned this pull request Nov 14, 2023

Roll Flutter from f66215027e14 to e8c2bb135659 (66 revisions) flutter/packages#5399

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2023

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

647daad

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024

7a278ae CommandResultEvent migrated (flutter/flutter#138165)

0e8302f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`CommandResultEvent` migrated#138165

`CommandResultEvent` migrated#138165
auto-submit[bot] merged 5 commits intoflutter:masterfrom
eliasyishak:ga4-migrate-command-result

eliasyishak commented Nov 9, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

eliasyishak Nov 9, 2023

Uh oh!

eliasyishak Nov 9, 2023

Uh oh!

christopherfujino Nov 9, 2023 •

edited

Loading

Uh oh!

eliasyishak Nov 9, 2023

Uh oh!

christopherfujino Nov 9, 2023 •

edited

Loading

Uh oh!

christopherfujino left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

eliasyishak commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Uh oh!

Uh oh!

eliasyishak Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

eliasyishak Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

christopherfujino Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eliasyishak Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

christopherfujino Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eliasyishak commented Nov 9, 2023 •

edited

Loading

christopherfujino Nov 9, 2023 •

edited

Loading

christopherfujino Nov 9, 2023 •

edited

Loading