New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use enums for telemetry event names #3992
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3992 +/- ##
========================================
- Coverage 78% 55% -23%
========================================
Files 406 327 -79
Lines 18929 13770 -5159
Branches 3034 2137 -897
========================================
- Hits 14659 7486 -7173
- Misses 4268 5793 +1525
- Partials 2 491 +489
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #3992 +/- ##
=======================================
+ Coverage 79% 79% +1%
=======================================
Files 409 410 +1
Lines 19024 19124 +100
Branches 3035 3052 +17
=======================================
+ Hits 14845 14926 +81
- Misses 4177 4190 +13
- Partials 2 8 +6
|
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.
I have a question about this change. Why wouldn't we also consider changing /src/telemetry/index.ts to make the captureTelemetry
function look like so?
export function captureTelemetry(
eventName: EventName, // <= take advantage of this new enum...
properties?: TelemetryProperties,
...
It's not strictly necessary to make this change of course, but I expected to see this and was wondering why it isn't there.
(This also applies to the other exported functions in .../telemetry/index.ts
: sendTelemetryEvent
and sendTelemetryWhenDone
)
That's part 2, |
c9d39c3
to
008268d
Compare
Please re-review |
008268d
to
f4b43c7
Compare
For #2904 (part 1 for #2904)
package-lock.json
has been regenerated by runningnpm install
(if dependencies have changed)