Skip to content

[tool] Remove all usages of the deprecated usage package #151518

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.

Sign up for GitHub

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

Jump to bottom
Closed
andrewkolos wants to merge 16 commits into from
Closed

[tool] Remove all usages of the deprecated usage package #151518

andrewkolos wants to merge 16 commits into from

Conversation

@andrewkolos
Copy link
Contributor

@andrewkolos andrewkolos commented Jul 10, 2024 •
edited
Loading

Helps #150575.

Also fixes #150665.
Also fixes #150663.
Also fixes #151560.

This is a recreation of #150667. I was originally going to split this up into smaller PRs, but this has proven very time-consuming and is prone to accidentally overwriting changes that land in between the smaller PRs.

Pre-launch Checklist

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

@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop labels Jul 10, 2024
@andrewkolos andrewkolos marked this pull request as ready for review July 10, 2024 18:29
@andrewkolos andrewkolos mentioned this pull request Jul 10, 2024
Open
christopherfujino
christopherfujino reviewed Jul 10, 2024
View reviewed changes
@andrewkolos andrewkolos force-pushed the remove-usage-of-usage branch 3 times, most recently from f2b493b to 7427786 Compare July 15, 2024 21:04
@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. a: internationalization Supporting other languages or locales. (aka i18n) d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos platform-web Web applications specifically f: integration_test The flutter/packages/integration_test plugin and removed a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. a: internationalization Supporting other languages or locales. (aka i18n) d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos platform-web Web applications specifically labels Jul 15, 2024
@andrewkolos andrewkolos force-pushed the remove-usage-of-usage branch from 9d282f0 to ad44d04 Compare August 22, 2024 16:54
@andrewkolos andrewkolos force-pushed the remove-usage-of-usage branch from ad44d04 to 0afffb4 Compare August 22, 2024 16:55
chingjun
chingjun requested changes Aug 22, 2024
View reviewed changes
Copy link
Contributor

@chingjun chingjun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Internally we are still depending on the code removed in this PR for analytics. We need to migrate it to use the new Analytics class before removing package:usage here.

@andrewkolos
Copy link
Contributor Author

andrewkolos commented Aug 22, 2024

Internally we are still depending on the code removed in this PR for analytics. We need to migrate it to use the new Analytics class before removing package:usage here.

This PR shouldn't remove any APIs that g3 needs, it should only remove the usage of those APIs within the tool itself.

@chingjun
Copy link
Contributor

chingjun commented Aug 22, 2024

Internally we are still depending on the code removed in this PR for analytics. We need to migrate it to use the new Analytics class before removing package:usage here.

This PR shouldn't remove any APIs that g3 needs, it should only remove the usage of those APIs within the tool itself.

The UsageEvent().send(), HotEvent().send etc. that are removed in this PR, internally we hook up to that for analytics.

@andrewkolos
Copy link
Contributor Author

andrewkolos commented Aug 22, 2024 •
edited
Loading

Internally we are still depending on the code removed in this PR for analytics. We need to migrate it to use the new Analytics class before removing package:usage here.

This PR shouldn't remove any APIs that g3 needs, it should only remove the usage of those APIs within the tool itself.

The UsageEvent().send(), HotEvent().send etc. that are removed in this PR, internally we hook up to that for analytics.

Oh. So these calls, which are effectively no-ops for open-source flutter since GA3 is offline, are not no-ops for g3 flutter?

@andrewkolos
Copy link
Contributor Author

andrewkolos commented Aug 22, 2024

The UsageEvent().send(), HotEvent().send etc. that are removed in this PR, internally we hook up to that for analytics.

Could you please link these? I am not sure if you are referring to call sites or definitions here.

@andrewkolos
Copy link
Contributor Author

andrewkolos commented Aug 22, 2024

One example I see is that g3 depends on overriding FlutterCommand.usageValues

@andrewkolos
Copy link
Contributor Author

andrewkolos commented Aug 22, 2024

Closing since I don't have the bandwidth to continue work on this. I will update the parent issue.

@andrewkolos andrewkolos reopened this Dec 2, 2024
@github-actions github-actions bot removed the a: text input Entering text in a text field or keyboard related problems label Dec 2, 2024
@andrewkolos andrewkolos mentioned this pull request Dec 3, 2024
Merged
9 tasks
andrewkolos
andrewkolos commented Dec 3, 2024
View reviewed changes
dev/devicelab/bin/tasks/build_android_host_app_with_module_aar.dart
environment: <String, String>{
'JAVA_HOME': javaHome,
'FLUTTER_ANALYTICS_LOG_FILE': analyticsOutputFile.path,
'FLUTTER_SUPPRESS_ANALYTICS': 'true',
Copy link
Contributor Author

@andrewkolos andrewkolos Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt FLUTTER_SUPPRESS_ANALYTICS is actually needed since we have bot detection elsewhere, but I would rather delete all of these in a separate PR.

github-merge-queue bot pushed a commit that referenced this pull request Dec 5, 2024
@andrewkolos
Remove some usages of package:usage (#159705)
1856940 
Split-off from #151518. Helps
#150575


<details>

<summary> Pre-launch checklist </summary> 


- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

</details>


<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
@devoncarew devoncarew mentioned this pull request Jan 3, 2025
Merged
9 tasks
@andrewkolos andrewkolos mentioned this pull request Jan 29, 2025
Open
3 tasks
@andrewkolos
Copy link
Contributor Author

andrewkolos commented Jan 29, 2025 •
edited
Loading

For archeologists: this was before the monorepo merge and before we got the OK to delete g3 dependence on package:usage. This entire PR, the review, and my understanding of things went stale by the time I was ready to pick this up again, so this was since replaced by smaller PRs. See parent tracking issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kevmoo kevmoo kevmoo left review comments

@chingjun chingjun chingjun requested changes

+1 more reviewer

@christopherfujino christopherfujino christopherfujino approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: desktop Running on desktop platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

5 participants

@andrewkolos @kevmoo @christopherfujino @cbracken @chingjun