send begin/end notifications when installing types packages#12551
Uh oh!
There was an error while loading. Please reload this page.
Conversation
| } | ||
| export interface InstallTypesEventBody{ | ||
| /** |
There was a problem hiding this comment.
missing typingsInstallerVersion.
There was a problem hiding this comment.
this event should be consumed only by editor so this information is unnecessary.
| if (!this.eventSender){ | ||
| return; | ||
| } | ||
| if (this.telemetryEnabled){ |
There was a problem hiding this comment.
why do we still need TypingsInstalledTelemetryEvent*?
There was a problem hiding this comment.
reasoning behind this was: these events are semantically different. one is intended to be consumed only in the editor, another - send by VSCode on our behalf. In theory the latter one can be sent by just grabbing the payload and sending it as is treating its content as opaque. Yes, in this case VSCode can reconstruct telemetry event from the content of End notification, however I'd prefer to keep them separate for simplify consumption. // cc @mjbvz for his opinion
There was a problem hiding this comment.
Yes, I think that keeping both makes sense since they are used for different purposes.
mjbvz left a comment
mjbvz left a comment There was a problem hiding this comment.
Thanks for the quick followup here. The new event api looks good to me. Once this is merged in, please just let me know when it be be available.
fixes#12540
// cc @mhegazy
open question: do we assume that existing consumers of tsserver will ignore new event or should we allow new events under the command line flag