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

docs: add semaphore example #6038

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Conversation

songmuhan
Copy link
Contributor

@songmuhan songmuhan commented Sep 29, 2023

Motivation

Add last example to semaphore described at this issue

Solution

A simple example that prevent several tests from running at the same time, using a static semaphore with one permit.

Rendered

@github-actions github-actions bot added the R-loom-sync Run loom sync tests on this PR label Sep 29, 2023
@Darksonn Darksonn added T-docs Topic: documentation A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels Sep 29, 2023
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.

Thank you for the PR. Overall, I think the example is good. I have some comments to improve the phrasing.

tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
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.

Thanks for the changes! They look good. I have a few more grammar suggestions, and some minor additions to the tests.

tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
@songmuhan songmuhan force-pushed the semaphore-example branch 2 times, most recently from 6b24580 to b6fd79f Compare October 1, 2023 09:27
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.

Please double-check how the documentation looks when rendered. You can find the command for doing that in the CONTRIBUTING.md file, or you can push and let our CI setup render it. The result is visible at: https://deploy-preview-6038--tokio-rs.netlify.app/tokio/sync/struct.semaphore

tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
@songmuhan songmuhan force-pushed the semaphore-example branch 3 times, most recently from bf50e91 to 8f6733e Compare October 5, 2023 10:05
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.

Thanks. This looks good to me.

@Darksonn Darksonn merged commit 5d29136 into tokio-rs:master Oct 5, 2023
100 of 130 checks passed
@songmuhan
Copy link
Contributor Author

Thank you for your precious time :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-sync Module: tokio/sync R-loom-sync Run loom sync tests on this PR T-docs Topic: documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants