-
Notifications
You must be signed in to change notification settings - Fork 151
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
Drop Configuration classes from API #2596
Conversation
BenchmarksBenchmark execution time: 2024-03-27 12:17:31 Comparing candidate commit c0cda03 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 177 metrics, 0 unstable metrics. scenario:TraceSerializationBench/benchSerializeTrace
|
85f4a09
to
5a8291d
Compare
They're nowhere used internally (anymore). Avoid having multiple APIs: dd_trace_env_config() is enough.
5a8291d
to
89e74d2
Compare
We generally accept a double internally (e.g. start_span), so this won't be needed for users either.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## ddtrace-1.0.0 #2596 +/- ##
===================================================
+ Coverage 77.51% 77.98% +0.46%
+ Complexity 2257 2181 -76
===================================================
Files 227 224 -3
Lines 25840 25676 -164
Branches 976 976
===================================================
- Hits 20030 20023 -7
+ Misses 5290 5133 -157
Partials 520 520
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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.
These removals look good to be as seemingly not used internally and not customer-facing.
FYI, this test is failing - I don't think it's related with this PR though (or maybe a re-run is enough, idk).
Description
They're nowhere used internally (anymore). Avoid having multiple APIs: dd_trace_env_config() is enough.
Please consider whether this makes sense.
Reviewer checklist