Skip to content

Conversation

d13
Copy link
Member

@d13 d13 commented Oct 4, 2024

  • Adds new telemetry events for graph
  • Adds new telemetry events for visual file history
  • Adds type to the webview descriptor and used as a key in new showAborted and shown events
  • Refactors onShowing hook to return a tuple which adds an optional context record that is mixed-in to the new show* events
  • Adds telemetry context into the webview app and a low-level custom event handler

@d13
Copy link
Member Author

d13 commented Oct 4, 2024

Hey @d13, while reviewing your PR, I'd suggest the following code changes:

👉 Option 1: generic webview show event

The idea here is that the controller will transparently handle the open attempted and open successful telemetry events for the webview

You can also review and apply these suggestions locally on your machine.

Learn more about GitKraken Code Suggest

Code Suggest liberates your code reviews from GitHub's restrictive, comment-only feedback style. As simple as suggesting changes in a Google-doc, provide real code suggestions from where you code, e.g. your IDE, and on anything in your project — not just on the lines of code changed in the PR.

Join your team on GitKraken to speed up PR review.

@d13
Copy link
Member Author

d13 commented Oct 4, 2024

Hey @d13, while reviewing your PR, I'd suggest the following code changes:

👉 **Option 2: from the WebviewProvider **

The idea here is similar where the controller will transparently capture the timing, but the WebviewProvider will handle the open attempted and open successful telemetry events. This leaves room for more control over what the webview wants to send, if anything at all.

You can also review and apply these suggestions locally on your machine.

Learn more about GitKraken Code Suggest

Code Suggest liberates your code reviews from GitHub's restrictive, comment-only feedback style. As simple as suggesting changes in a Google-doc, provide real code suggestions from where you code, e.g. your IDE, and on anything in your project — not just on the lines of code changed in the PR.

Join your team on GitKraken to speed up PR review.

@d13 d13 force-pushed the feature/webview-sends-telemetry branch from 12a538b to 7549b6f Compare October 7, 2024 16:18
@d13 d13 marked this pull request as ready for review October 7, 2024 19:48
@d13 d13 force-pushed the feature/webview-sends-telemetry branch from 5bd6ada to 4376e5e Compare October 8, 2024 00:51
- expands telemetry data captured in graph
- adds getTelemetryContext in provider and host

Co-authored-by: Eric Amodio <[email protected]>
@d13 d13 force-pushed the feature/webview-sends-telemetry branch from 4376e5e to 11fc243 Compare October 8, 2024 00:53
@d13 d13 merged commit 523bb12 into main Oct 8, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant