-
Notifications
You must be signed in to change notification settings - Fork 577
improvement(client-telemetry-utils): BREAKING CHANGE: type erase ITelemetryLoggerExt
#27476
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| "@fluidframework/telemetry-utils": minor | ||
| "__section": breaking | ||
| --- | ||
| Deprecated ITelemetryLoggerExt methods and related types removed | ||
|
|
||
| Methods on `ITelemetryLoggerExt` are removed except for `send` inherited from `ITelemetryBaseLogger`. | ||
|
|
||
| See issue [#26910](https://github.com/microsoft/FluidFramework/issues/26910) for complete details. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,13 +6,11 @@ | |
| // eslint-disable-next-line no-restricted-syntax | ||
| export * from "./main.js"; | ||
|
|
||
| // Additional APIs that are deprecated and thus left out of the common export set. | ||
| // Additional APIs that are special and thus left out of the common export set. | ||
| export type { | ||
| ITelemetryErrorEventExt, | ||
| ITelemetryGenericEventExt, | ||
| // ITelemetryLoggerExt is temporarily set to an alias of `TelemetryLoggerExt` when imported from "/internal". | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When can this be removed? We should probably be explicit about that in the comment here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whenever. Right now or later. @MarioJGMsoft has AB#74860. Likely it will happen before this PR merges, but if not, I'll add reference in here.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened this PR to remove the code: #27484 |
||
| // Add it to "/legacy" export set here. | ||
| ITelemetryLoggerExt, | ||
| ITelemetryPerformanceEventExt, | ||
| TelemetryEventCategory, | ||
| } from "./telemetryTypes.js"; | ||
|
|
||
| // ---------------------------------------------------------------------------- | ||
|
|
||
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.
Do we have a backlog item tracking this cleanup? Would be nice if all of the relevant temporary export notes could include a TODO note referencing that item. Would make it easier to find and cleanup everything relevant when we tackle that item.
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'm already doing the cleanup here: #27484