-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[service] Remove servicetelemetry.TelemetrySettings #10728
[service] Remove servicetelemetry.TelemetrySettings #10728
Conversation
dae58da
to
5aaad3c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10728 +/- ##
=======================================
Coverage 92.18% 92.18%
=======================================
Files 403 401 -2
Lines 18792 18786 -6
=======================================
- Hits 17323 17318 -5
+ Misses 1109 1108 -1
Partials 360 360 ☔ View full report in Codecov by Sentry. |
# Use this changelog template to create an entry for release notes. | ||
|
||
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
change_type: breaking |
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 not been able to find any public references to this symbol being used as an argument to a function, so I think it's okay to do in one go
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, this feels less ambiguous.
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.
LGTM. Nice to see servicetelemetry.TelemetrySettings
gone.
Description
Reorganizes service to not require
servicetelemetry.TelemetrySettings
and instead depend directly oncomponent.TelemetrySettings
Whether or not we move forward with #10725 I think this is a useful change for service.
Testing
Unit tests