-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
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
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 42216813+eliasyishak@users.noreply.github.com Update helper message for `--suppress-analytics` (flutter/flutter#124810) 2023-04-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8b7cdb02f7f3 to 609f9d536494 (1 revision) (flutter/flutter#125097) 2023-04-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from 099ed6c62d04 to 8b7cdb02f7f3 (6 revisions) (flutter/flutter#125094) 2023-04-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5fcc7b719029 to 099ed6c62d04 (3 revisions) (flutter/flutter#125078) 2023-04-19 jmccandless@google.com Disableable ContextMenuButtonItems (flutter/flutter#124253) 2023-04-18 58190796+MitchellGoodwin@users.noreply.github.com Adaptive alert dialog (flutter/flutter#124336) 2023-04-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6d263ea56a62 to 5fcc7b719029 (4 revisions) (flutter/flutter#125069) 2023-04-18 58529443+srujzs@users.noreply.github.com Remove package:js/dart:js_interop conflicts (flutter/flutter#124879) 2023-04-18 abadasamuelosp@gmail.com Remove double.fromEnvironment from dart-define doc (flutter/flutter#124102) 2023-04-18 40026920+KKimj@users.noreply.github.com Update to add Kim Jiun to `AUTHORS` (flutter/flutter#125026) 2023-04-18 gspencergoog@users.noreply.github.com Add controller argument to SubmenuButton (flutter/flutter#125000) 2023-04-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 879308a52228 to 6d263ea56a62 (1 revision) (flutter/flutter#125060) 2023-04-18 jmccandless@google.com Limit the number of Material spell check suggestions to 3 (flutter/flutter#124899) 2023-04-18 magder@google.com Remove impeller testowners (flutter/flutter#125056) 2023-04-18 110993981+htoor3@users.noreply.github.com [web] - Clean up skipped tests (flutter/flutter#124981) 2023-04-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 72b68622fffa to 879308a52228 (1 revision) (flutter/flutter#125057) 2023-04-18 goderbauer@google.com Remove unused getRootRenderObject and getSelectedRenderObject service extensions (flutter/flutter#124805) 2023-04-18 thkim1011@users.noreply.github.com l10n.yaml's nullable-getter option should default to true (flutter/flutter#124353) 2023-04-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 55bb065c607b to 72b68622fffa (1 revision) (flutter/flutter#125053) 2023-04-18 47866232+chunhtai@users.noreply.github.com 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 rmistry@google.com,stuartmorgan@google.com,ychris@google.com 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
…r#3760) flutter/flutter@42fb0b2...3476b96 2023-04-19 42216813+eliasyishak@users.noreply.github.com Update helper message for `--suppress-analytics` (flutter/flutter#124810) 2023-04-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8b7cdb02f7f3 to 609f9d536494 (1 revision) (flutter/flutter#125097) 2023-04-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from 099ed6c62d04 to 8b7cdb02f7f3 (6 revisions) (flutter/flutter#125094) 2023-04-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5fcc7b719029 to 099ed6c62d04 (3 revisions) (flutter/flutter#125078) 2023-04-19 jmccandless@google.com Disableable ContextMenuButtonItems (flutter/flutter#124253) 2023-04-18 58190796+MitchellGoodwin@users.noreply.github.com Adaptive alert dialog (flutter/flutter#124336) 2023-04-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6d263ea56a62 to 5fcc7b719029 (4 revisions) (flutter/flutter#125069) 2023-04-18 58529443+srujzs@users.noreply.github.com Remove package:js/dart:js_interop conflicts (flutter/flutter#124879) 2023-04-18 abadasamuelosp@gmail.com Remove double.fromEnvironment from dart-define doc (flutter/flutter#124102) 2023-04-18 40026920+KKimj@users.noreply.github.com Update to add Kim Jiun to `AUTHORS` (flutter/flutter#125026) 2023-04-18 gspencergoog@users.noreply.github.com Add controller argument to SubmenuButton (flutter/flutter#125000) 2023-04-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 879308a52228 to 6d263ea56a62 (1 revision) (flutter/flutter#125060) 2023-04-18 jmccandless@google.com Limit the number of Material spell check suggestions to 3 (flutter/flutter#124899) 2023-04-18 magder@google.com Remove impeller testowners (flutter/flutter#125056) 2023-04-18 110993981+htoor3@users.noreply.github.com [web] - Clean up skipped tests (flutter/flutter#124981) 2023-04-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 72b68622fffa to 879308a52228 (1 revision) (flutter/flutter#125057) 2023-04-18 goderbauer@google.com Remove unused getRootRenderObject and getSelectedRenderObject service extensions (flutter/flutter#124805) 2023-04-18 thkim1011@users.noreply.github.com l10n.yaml's nullable-getter option should default to true (flutter/flutter#124353) 2023-04-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 55bb065c607b to 72b68622fffa (1 revision) (flutter/flutter#125053) 2023-04-18 47866232+chunhtai@users.noreply.github.com 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 rmistry@google.com,stuartmorgan@google.com,ychris@google.com 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.