Skip to content

Bhs/scopes update#2

Open
carlosalberto wants to merge 20 commits intobhs:bhs/scopesfrom
carlosalberto:bhs/scopes-update
Open

Bhs/scopes update#2
carlosalberto wants to merge 20 commits intobhs:bhs/scopesfrom
carlosalberto:bhs/scopes-update

Conversation

@carlosalberto
Copy link
Copy Markdown
Collaborator

Update the latest examples to the Scope prototype.

Observe it can't be merged automatically (I had to manually fix the merge from contrib/master into this branch ;) ) For your consideration, and wondering if I should put this branch in the opentracing-contrib repo itself (not in master, of course ;) )

private final AutoFinishScope.Continuation continuation;

Callback(Scope activeSpan) {
continuation = ((AutoFinishScope)activeSpan).defer();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

casting doesn't look nice. What if it's not AutoFinishScope?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hey!

So, if you are talking about the chance of Scope not being a AutoFinishScope here, I can add the check for sanity purposes (even though we know that for this example, we always get an Auto one :) )

But if you are talking about doing such cast overall - that's a reason to try to include the defer and Continuation api into the main interface (i.e. io.opentracing.Scope)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

yeah, my concern about generic use case

@carlosalberto carlosalberto force-pushed the bhs/scopes-update branch 2 times, most recently from e7a67e6 to f3dded7 Compare August 23, 2017 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants