-
Notifications
You must be signed in to change notification settings - Fork 847
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
Benchmark test in Typescript #4143
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4143 +/- ##
==========================================
- Coverage 92.72% 92.24% -0.48%
==========================================
Files 298 326 +28
Lines 8297 9299 +1002
Branches 1728 1971 +243
==========================================
+ Hits 7693 8578 +885
- Misses 604 721 +117 |
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.
Thanks for taking the time to create this PR. I think prefer this one. 🙂
cc @open-telemetry/javascript-approvers thoughts? I think this is here a bit more seamless with our other code also written typescript 🙂
I think I might prefer JS actually in this context. If we update our version of typescript it might change the benchmarks. It's ok if it changes the performance of the library because that's what we're measuring, but we want the thing we use to measure performance to be as stable as possible. If we update TS and the benchmark gets 5% slower is that because the library got slower or because the benchmark got slower (admittedly a synthetic example, but i think it gets the point across)? |
Yep, that makes sense. @martinkuba thanks for putting in the effort to open this PR and sorry for the churn - let's go with #4105 |
This is the same as #4105, but written in Typescript.