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

Improve LocalPoolHandle #4680

Merged
merged 8 commits into from Jun 17, 2022
Merged

Improve LocalPoolHandle #4680

merged 8 commits into from Jun 17, 2022

Conversation

b-naber
Copy link
Contributor

@b-naber b-naber commented May 11, 2022

Adds some functionality to LocalPoolHandle and tries to improve existing docs.

Fixes #4661

@b-naber b-naber changed the title Local pool handle Improve LocalPoolHandle May 11, 2022
@Darksonn Darksonn added A-tokio-util Area: The tokio-util crate M-task Module: tokio/task labels May 11, 2022
tokio-util/src/task/spawn_pinned.rs Outdated Show resolved Hide resolved
tokio-util/src/task/spawn_pinned.rs Outdated Show resolved Hide resolved
tokio-util/src/task/spawn_pinned.rs Outdated Show resolved Hide resolved
tokio-util/tests/spawn_pinned.rs Outdated Show resolved Hide resolved
tokio-util/tests/spawn_pinned.rs Outdated Show resolved Hide resolved
@b-naber b-naber force-pushed the local-pool-handle branch 2 times, most recently from 2c23c9c to aca3e04 Compare May 12, 2022 09:09
@b-naber
Copy link
Contributor Author

b-naber commented May 12, 2022

@Darksonn Addressed your review.

tokio-util/tests/spawn_pinned.rs Outdated Show resolved Hide resolved
tokio-util/src/task/spawn_pinned.rs Outdated Show resolved Hide resolved
@netlify
Copy link

netlify bot commented May 18, 2022

Deploy Preview for tokio-rs ready!

Name Link
🔨 Latest commit 1368c20
🔍 Latest deploy log https://app.netlify.com/sites/tokio-rs/deploys/6284f44da36a9b000ac59dc4
😎 Deploy Preview https://deploy-preview-4680--tokio-rs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@b-naber
Copy link
Contributor Author

b-naber commented May 18, 2022

@Darksonn Updated the PR.

@Darksonn
Copy link
Contributor

Your doc-tests don't compile.

Comment on lines 164 to 166
/// let _ = handles
/// .into_iter()
/// .map(|handle| async { handle.await.unwrap() });
Copy link
Contributor

@Darksonn Darksonn May 29, 2022

Choose a reason for hiding this comment

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

This doesn't actually await them because iterators are lazy. Just use a for loop over handles instead.

Suggested change
/// let _ = handles
/// .into_iter()
/// .map(|handle| async { handle.await.unwrap() });
/// for handle in handles {
/// handle.await.unwrap();
/// }

@Darksonn
Copy link
Contributor

I believe everything this is missing is a bit of documentation stuff. Are you able to finish the PR?

@b-naber
Copy link
Contributor Author

b-naber commented Jun 17, 2022

I believe everything this is missing is a bit of documentation stuff. Are you able to finish the PR?

I forgot that you replied to this, sorry about that. Is there anything else that needs to be changed?

Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Darksonn Darksonn merged commit d8fb721 into tokio-rs:master Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-util Area: The tokio-util crate M-task Module: tokio/task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flesh out LocalPoolHandle
2 participants