-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Update helper message for --suppress-analytics
#124810
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
Update helper message for --suppress-analytics
#124810
Conversation
…g-and-suppressing-analytics
|
Can you also update the printStatus in packages/flutter_tools/lib/runner.dart to only suggest using |
| // If the user has opted out of legacy analytics, we will continue | ||
| // to opt them out of unified analytics and inform them | ||
| if (!globals.flutterUsage.enabled && globals.analytics.telemetryEnabled) { | ||
| await globals.analytics.setTelemetry(false); | ||
| globals.logger.printStatus( | ||
| 'Please note that analytics reporting was already disabled, and will continue to be disabled.\n'); | ||
| } | ||
|
|
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.
fyi @christopherfujino , the unified analytics implementation team decided that we should have the package be responsible for checking the legacy opt out status.
This PR (dart-lang/tools#80) has the changes to pass legacy opt out status to unified analytics.
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.
Ok, so I'm guessing landing this PR is blocked on rolling package:unified_analytics?
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.
Yep, I'm going to confirm that the package is rolled first before landing this
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.
Actually, we have to ensure that the new version of package:unified_analytics has rolled to the analyzer, which has rolled to the Dart SDK, which has rolled to the engine, which has rolled to the framework, right?
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.
We have bumped the hash on the dart sdk side in the DEPS file now, does this PR #125005 mean that the package update has made it to the framework now?
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.
I'm seeing in g3 that the latest version for the package has actually been rolled so i think once the package has been updated in #125005, then we can land this PR
|
|
||
| testUsingContext( | ||
| 'legacy analytics disabled will disable new analytics', | ||
| () async { | ||
|
|
||
| io.setExitFunctionForTests((int exitCode) {}); | ||
|
|
||
| await runner.run( | ||
| <String>[], | ||
| () => <FlutterCommand>[], | ||
| // This flutterVersion disables crash reporting. | ||
| flutterVersion: '[user-branch]/', | ||
| shutdownHooks: ShutdownHooks(), | ||
| ); | ||
|
|
||
| expect(globals.flutterUsage.enabled, false); | ||
| expect(globals.analytics.telemetryEnabled, false); | ||
| expect(testLogger.statusText.contains( | ||
| 'Please note that analytics ' | ||
| 'reporting was already disabled'), true); | ||
| }, | ||
| overrides: <Type, Generator>{ | ||
| Analytics: () => FakeAnalytics(), | ||
| FileSystem: () => MemoryFileSystem.test(), | ||
| ProcessManager: () => FakeProcessManager.any(), | ||
| Usage: () => legacyAnalytics, | ||
| }, | ||
| ); |
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.
This test was no longer necessary since legacy opt out status will be honored from within package:unified_analytics for every tool using that package
packages/flutter_tools/lib/src/runner/flutter_command_runner.dart
Outdated
Show resolved
Hide resolved
christopherfujino
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.
Two nits about messaging text, otherwise LGTM
…g-and-suppressing-analytics
This reverts commit 3476b96.
flutter/flutter@42fb0b2...3476b96 2023-04-19 [email protected] Update helper message for `--suppress-analytics` (flutter/flutter#124810) 2023-04-19 [email protected] Roll Flutter Engine from 8b7cdb02f7f3 to 609f9d536494 (1 revision) (flutter/flutter#125097) 2023-04-19 [email protected] Roll Flutter Engine from 099ed6c62d04 to 8b7cdb02f7f3 (6 revisions) (flutter/flutter#125094) 2023-04-19 [email protected] Roll Flutter Engine from 5fcc7b719029 to 099ed6c62d04 (3 revisions) (flutter/flutter#125078) 2023-04-19 [email protected] Disableable ContextMenuButtonItems (flutter/flutter#124253) 2023-04-18 [email protected] Adaptive alert dialog (flutter/flutter#124336) 2023-04-18 [email protected] Roll Flutter Engine from 6d263ea56a62 to 5fcc7b719029 (4 revisions) (flutter/flutter#125069) 2023-04-18 [email protected] Remove package:js/dart:js_interop conflicts (flutter/flutter#124879) 2023-04-18 [email protected] Remove double.fromEnvironment from dart-define doc (flutter/flutter#124102) 2023-04-18 [email protected] Update to add Kim Jiun to `AUTHORS` (flutter/flutter#125026) 2023-04-18 [email protected] Add controller argument to SubmenuButton (flutter/flutter#125000) 2023-04-18 [email protected] Roll Flutter Engine from 879308a52228 to 6d263ea56a62 (1 revision) (flutter/flutter#125060) 2023-04-18 [email protected] Limit the number of Material spell check suggestions to 3 (flutter/flutter#124899) 2023-04-18 [email protected] Remove impeller testowners (flutter/flutter#125056) 2023-04-18 [email protected] [web] - Clean up skipped tests (flutter/flutter#124981) 2023-04-18 [email protected] Roll Flutter Engine from 72b68622fffa to 879308a52228 (1 revision) (flutter/flutter#125057) 2023-04-18 [email protected] Remove unused getRootRenderObject and getSelectedRenderObject service extensions (flutter/flutter#124805) 2023-04-18 [email protected] l10n.yaml's nullable-getter option should default to true (flutter/flutter#124353) 2023-04-18 [email protected] Roll Flutter Engine from 55bb065c607b to 72b68622fffa (1 revision) (flutter/flutter#125053) 2023-04-18 [email protected] Add vmservice for android build options (flutter/flutter#123034) 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 Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/flutter@42fb0b2...3476b96 2023-04-19 [email protected] Update helper message for `--suppress-analytics` (flutter/flutter#124810) 2023-04-19 [email protected] Roll Flutter Engine from 8b7cdb02f7f3 to 609f9d536494 (1 revision) (flutter/flutter#125097) 2023-04-19 [email protected] Roll Flutter Engine from 099ed6c62d04 to 8b7cdb02f7f3 (6 revisions) (flutter/flutter#125094) 2023-04-19 [email protected] Roll Flutter Engine from 5fcc7b719029 to 099ed6c62d04 (3 revisions) (flutter/flutter#125078) 2023-04-19 [email protected] Disableable ContextMenuButtonItems (flutter/flutter#124253) 2023-04-18 [email protected] Adaptive alert dialog (flutter/flutter#124336) 2023-04-18 [email protected] Roll Flutter Engine from 6d263ea56a62 to 5fcc7b719029 (4 revisions) (flutter/flutter#125069) 2023-04-18 [email protected] Remove package:js/dart:js_interop conflicts (flutter/flutter#124879) 2023-04-18 [email protected] Remove double.fromEnvironment from dart-define doc (flutter/flutter#124102) 2023-04-18 [email protected] Update to add Kim Jiun to `AUTHORS` (flutter/flutter#125026) 2023-04-18 [email protected] Add controller argument to SubmenuButton (flutter/flutter#125000) 2023-04-18 [email protected] Roll Flutter Engine from 879308a52228 to 6d263ea56a62 (1 revision) (flutter/flutter#125060) 2023-04-18 [email protected] Limit the number of Material spell check suggestions to 3 (flutter/flutter#124899) 2023-04-18 [email protected] Remove impeller testowners (flutter/flutter#125056) 2023-04-18 [email protected] [web] - Clean up skipped tests (flutter/flutter#124981) 2023-04-18 [email protected] Roll Flutter Engine from 72b68622fffa to 879308a52228 (1 revision) (flutter/flutter#125057) 2023-04-18 [email protected] Remove unused getRootRenderObject and getSelectedRenderObject service extensions (flutter/flutter#124805) 2023-04-18 [email protected] l10n.yaml's nullable-getter option should default to true (flutter/flutter#124353) 2023-04-18 [email protected] Roll Flutter Engine from 55bb065c607b to 72b68622fffa (1 revision) (flutter/flutter#125053) 2023-04-18 [email protected] Add vmservice for android build options (flutter/flutter#123034) 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 Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes: #124808
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.