-
Notifications
You must be signed in to change notification settings - Fork 48
Added telemetry to typescript side of extension #211
Conversation
src/extension.ts
Outdated
@@ -172,7 +172,20 @@ export async function activate(context: vscode.ExtensionContext) { | |||
switch (message.command) { | |||
case WEBVIEW_MESSAGES.BUTTON_PRESS: | |||
// Send input to the Python process | |||
handleButtonPressTelemetry(message.text); | |||
switch (currentActiveDevice) { |
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.
wondering if there is a better way to do this, then switch cases everywhere. It might become heavy when we have more devices. Potentially wrapper function or maybe adding an argument to the telemetry event if that is possible
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.
great suggestion. I updated it. Lmk what u think
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.
+100 on the telemetry integration :D
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.
LGTM
Description:
Type of change
Please delete options that are not relevant.
Limitations:
Please describe limitations of this PR
Testing:
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist:
npm run format
and passes the checks innpm run check