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

Implement Runtime::block_on using oneshot #391

Merged
merged 3 commits into from
Jun 5, 2018

Conversation

jonhoo
Copy link
Sponsor Contributor

@jonhoo jonhoo commented Jun 4, 2018

A new take on #328 in which the provided future is spawned on the runtime, and a oneshot channel is used to communicate the result back.

@carllerche
Copy link
Member

This is a fine initial impl. However, what do you think about removing the free fn initially and only provide the Runtime::block_on in this PR? In the past, fns like block_on have been the source of confusion. So, keeping it a bit hidden might be good until the confusion can be explored further.

@jonhoo
Copy link
Sponsor Contributor Author

jonhoo commented Jun 4, 2018

Sure, that sounds like a good idea. Do you want to just not export it, or remove the fn entirely?

@carllerche
Copy link
Member

I would remove it entirely for now and leave only Runtime::block_on.

@carllerche
Copy link
Member

Oh, I'd also ❤️ some tests 💃

Copying some of these would probably work: https://github.com/tokio-rs/tokio/blob/master/tests/current_thread.rs

@jonhoo
Copy link
Sponsor Contributor Author

jonhoo commented Jun 5, 2018

@carllerche Done and done!

@carllerche
Copy link
Member

Thanks 👍

@carllerche carllerche merged commit 3d7263d into tokio-rs:master Jun 5, 2018
@jonhoo jonhoo deleted the block_on branch June 5, 2018 18:36
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.

None yet

2 participants