[go: nahoru, domu]

Skip to content
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

eliasyishak
Copy link
Contributor
@eliasyishak eliasyishak commented Apr 13, 2023

Fixes: #124808

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.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Apr 13, 2023
@christopherfujino
Copy link
Member

Can you also update the printStatus in packages/flutter_tools/lib/runner.dart to only suggest using flutter --disable-telemetry?

Comment on lines -91 to -98
// 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');
}

Copy link
Contributor Author

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.

Copy link
Member

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?

Copy link
Contributor Author

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

Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Contributor Author

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

@eliasyishak eliasyishak marked this pull request as ready for review April 17, 2023 16:26
Comment on lines -349 to -376

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,
},
);
Copy link
Contributor Author

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

Copy link
Member
@christopherfujino christopherfujino left a 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

@eliasyishak eliasyishak added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 19, 2023
@auto-submit auto-submit bot merged commit 3476b96 into flutter:master Apr 19, 2023
118 of 119 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 19, 2023
godofredoc added a commit that referenced this pull request Apr 19, 2023
auto-submit bot pushed a commit that referenced this pull request Apr 19, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Apr 19, 2023
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
auto-submit bot pushed a commit that referenced this pull request Apr 19, 2023
eliasyishak added a commit to eliasyishak/flutter that referenced this pull request Apr 24, 2023
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up help messages for disabling and suppressing analytics
2 participants