send begin/end notifications when installing types packages #12551
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.
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