-
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
Add Scheme to MapProvider interface #5068
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5068 +/- ##
=======================================
Coverage 90.33% 90.33%
=======================================
Files 182 182
Lines 11024 11032 +8
=======================================
+ Hits 9958 9966 +8
Misses 841 841
Partials 225 225
Continue to review full report at Codecov.
|
This is another required step in order to support configuring map providers via the Builder. Updates open-telemetry#4759 This is a breaking change, since adds a new func to an interface. It is hard to make this backwards compatible, and would like to move forward with this exception. Signed-off-by: Bogdan Drutu <[email protected]>
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, I find this to be an acceptable breakage of the rules (but would like to hear from at least one other approver)
Seems reasonable to me as well |
Does |
I have a different view of this, kind of the |
Yeah, I guess that works too. The factory-based approach would probably be a bit more uniform with what we do with components from naming perspective, but I think your approach is fine too. |
This is another required step in order to support configuring map providers via the Builder. Updates open-telemetry#4759 This is a breaking change, since adds a new func to an interface. It is hard to make this backwards compatible, and would like to move forward with this exception. Signed-off-by: Bogdan Drutu <[email protected]> Co-authored-by: Alex Boten <[email protected]>
This is another required step in order to support configuring map providers via the Builder.
Updates #4759
This is a breaking change, since adds a new func to an interface. It is hard to make this backwards compatible, and would like to move forward with this exception.
Signed-off-by: Bogdan Drutu [email protected]