Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Don't override the return type in buildSpan(). #372

Closed
wants to merge 2 commits into from

Conversation

mmimica
Copy link

@mmimica mmimica commented Feb 19, 2020

This makes it possible to override buildSpan(String op) of MockTracer and return another impl of Tracer.SpanBuilder.
A use-case for this would be, for example, to supply an instance of SpanBuilder that delegates to original MockTracer.SpanBuilder, but also performs additional logging.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.586% when pulling 6bc1d8a on mmimica:master into a8557d7 on opentracing:master.

@whiskeysierra
Copy link

The proper way of doing this would be composition, not inheritance. You can take a look at https://github.com/zalando/opentracing-toolbox/tree/master/opentracing-proxy to see an example.

@mmimica
Copy link
Author

mmimica commented Feb 19, 2020

The proper way of doing this would be composition, not inheritance.

Indeed.

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you extend MockTracer.SpanBuilder, then you do not need this change.

@mmimica
Copy link
Author

mmimica commented Feb 19, 2020

if you extend MockTracer.SpanBuilder, then you do not need this change.

My extension to SpanBuilder is generic and has nothing to do with mocks, so I'm not going do that.

@yurishkuro
Copy link
Member

if you're using MockTracer, you should be extending its subcomponents.

You're proposing a breaking change.

@mmimica mmimica closed this Feb 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants