-
Notifications
You must be signed in to change notification settings - Fork 319
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
Remove GA Analytics #7760
base: master
Are you sure you want to change the base?
Remove GA Analytics #7760
Conversation
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.
Do we really want to remove all this? I thought the plan was to replace legacy analytics with unified analytics events. Removing all the analytics event send sites would require that we add them all back with unified analytics instead of just swapping the logic to send through unified analytics package instead of through the legacy GA.
Acknowledged and we can discuss more if you want. The counter argument though is that the Analytics API was designed with a GA-first mind set, and especially as time went on, the APIs were not called consistently, nor everywhere where we want them called from. I imagine that this PR will be used when the better analytics goes in, but now we'll have a clean history to improve code health and ultimately a better analytics story for the plugin. |
Ack. Do we still show the unified analytics prompt coming from the analysis server after this change? CC @bwilkerson As long as we are not removing that logic, then it's probably fine to remove all this. |
The logic to notify the IDEs in a generic way for analytics goes through the DAS (non-flutter) protocol via the Dart plugin for IntelliJ. Relevant links:
|
54c5819
to
a4e59f3
Compare
Note: don't commit yet, the Flutter preference page has an issue with this PR, it needs to be fixed before this can land |
a4e59f3
to
14e4c93
Compare
No description provided.