-
Notifications
You must be signed in to change notification settings - Fork 810
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
feat(sdk-metrics)!: export factory functions over classes #4932
base: next
Are you sure you want to change the base?
feat(sdk-metrics)!: export factory functions over classes #4932
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #4932 +/- ##
=======================================
Coverage 93.24% 93.25%
=======================================
Files 318 318
Lines 8231 8239 +8
Branches 1651 1651
=======================================
+ Hits 7675 7683 +8
Misses 556 556
|
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.
Looking good.
Here are some more hits of the old usage in docs and examples to apply to your branch:
https://gist.github.com/trentm/889d78ad727c53aae1e10244768cd9a6
packages/sdk-metrics/test/export/PeriodicExportingMetricReader.test.ts
Outdated
Show resolved
Hide resolved
….test.ts Co-authored-by: Trent Mick <[email protected]>
docs: align docs and examples with changes
cdd22fe
to
e2a36a9
Compare
Thanks for pointing that out - there were actually a few more things that needed changing from the previous PRs I made for that package, I applied them all in e2a36a9 and a762455 |
Currently types of private properties end up being part of the public interface, which causes breaking changes left, right and center. Using a factory function we can only expose what we want the user to use, this enables us to make larger changes to SDK internals without breaking users.
Note: This is combined with #4931 as it also eliminates some classes that we previously exported.
Old:
New:
For a user it's actually a fairly minimal change but for us it means that we can freely modify the SDK's MeterProvider and all previously unintentionally public referenced and transitively referenced types.