[CP] Migrate command usage values (#139383) #141019
Conversation
Related to the tracker issue: - flutter#128251 This PR migrates the `Usage.command` static method that sent custom dimensions for each command (if applicable). The screenshot below shows the different places where the `usageValues` getter is overwritten to return the necessary custom dimensions for that command. <img width="285" alt="image" src="https://github.com/flutter/flutter/assets/42216813/e32d5100-0e17-4a4d-8f21-327a8c113a19">
github-actions
bot
added
the
tool
Per discord discussion, building an AAR out of a plugin project has not worked for years, so let's just disable the functionality. Context: flutter#137564
| if (!manifest.isModule && !manifest.isPlugin) { | ||
| throwToolExit('AARs can only be built for plugin or module projects.'); | ||
| if (!manifest.isModule) { | ||
| throwToolExit('AARs can only be built for module projects.'); |
There was a problem hiding this comment.
No, this was brought in from your commit actually. There was a change you made that my CP was depending on. I thought originally that I would need to only merge previously committed PRs for a CP.
It's probably best to revert the changes from your commit and just make the necessary changes for this PR to work right?
| 'This cannot currently be configured.'; | ||
|
|
||
| @override | ||
| Future<void> validateCommand() async { |
There was a problem hiding this comment.
same question, is this required for usage values?
There was a problem hiding this comment.
It is not required for this PR
There was a problem hiding this comment.
Well turns out this was necessary for this test 🙃
There was a problem hiding this comment.
Let's delete the test rather than change the behavior of the tool in CP.
…alytics (flutter#134756) Part of: - flutter#128251 Currently, when we want to use the analytics instance from `package:unified_analytics`, we are just grabbing it from globals. However, with the legacy analytics instance, there are some things we check to return a no-op version of the instance.. for example, if we are running on bots or a non standard branch, we use a no-op instance This PR uses the same previous checks for the new analytics instance
github-actions
bot
added
the
a: text input
|
@christopherfujino looks like this is ready for your review now |
| ); | ||
| }); | ||
|
|
||
| testUsingContext('will not build an AAR for a plugin', () async { |
There was a problem hiding this comment.
If the tested behavior did not already exist on the 3.16 branch, let's just delete this test, per https://github.com/flutter/flutter/pull/141019/files#r1445425246
christopherfujino
left a comment
christopherfujino
left a comment
There was a problem hiding this comment.
This LGTM once you remove the BuildAarCommand.validateCommand() override and the failing test.
itsjustkevin
added
the
autosubmit
3 participants
Fixes: