Skip to content
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

Added a Step extension to allow a step to specify its parent #38

Closed
wants to merge 1 commit into from

Conversation

adamtegen
Copy link
Contributor

Also updated the minithreaded console example to show how to consume it.

This allowed the sub-timings to show under their correct parent.

Also updated the minithreaded console example to show how to consume it.
@yellis
Copy link
Member

yellis commented Feb 24, 2014

What is the use case for this?

@adamtegen
Copy link
Contributor Author

This aids in multi-threaded scenarios. It "fixes" the multi-threaded console example

@yellis
Copy link
Member

yellis commented Feb 24, 2014

Ok.

Can you please remove the changes to the Step and StepImpl function return types (and rebase so that it doesnt show in the PR history)? We don't want to change these as part of the PR.

@adamtegen
Copy link
Contributor Author

The change on those return types was necessary because I needed to pass the Step into the SubStep function.

@yellis
Copy link
Member

yellis commented Feb 25, 2014

@NickCraver @dixon - what do you guys think about changing the return type of Step and StemImpl from IDisposable to Timing? Timing is the only type returned by these functions and it implements IDisposable, so in theory even though this is an API change, it wont break anyone's code.

@dixon
Copy link
Member

dixon commented Feb 27, 2014

I'll look at this PR this weekend.

@yellis
Copy link
Member

yellis commented Apr 2, 2014

@dixon you get a chance to check this out yet?

@NickCraver
Copy link
Member

I agree with the problem here, but not necessarily the solution. A better option is that multi-threaded access just works, which is a much harder problem, but a better overall goal of solving. I'm taking a look at that next in v4 - working on async now.

I don't disagree with the timing steps returning Timing though (for general use), I'll make that change in the current v4 tree.

@NickCraver NickCraver closed this Feb 18, 2017
@NickCraver NickCraver mentioned this pull request Feb 18, 2017
43 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants