-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fix the example in document for WaitTimeoutResult::timed_out #110056
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
Conversation
r? @thomcc (rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
fe4d622
to
523eca1
Compare
r? libs |
r? libs |
library/std/src/sync/condvar.rs
Outdated
/// This example spawns a thread which will sleep 20 milliseconds before | ||
/// updating the condvar and then notify. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where you're going seems like a good direction but currently it ends up at... "and then notify"... notify what?
library/std/src/sync/condvar.rs
Outdated
/// The main thread will wait with a timeout with only 10 milliseconds on the condvar | ||
/// and will leave once timeout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"with a timeout with"... This could be constructed more like "with a 10 millisecond timeout".
And what is the thread "leaving"? The loop, right? But it could be clearer.
@rustbot author |
78fee78
to
5d11c20
Compare
@rustbot ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better! Just a little bit more?
Side thought: It'd be cute if this example showed that the condvar's timeout and notification status can differ, but that's not a requirement. This still addresses the concern in question. |
a1f5939
to
d67d989
Compare
@bors r+ rollup |
…rkingjubilee Fix the example in document for WaitTimeoutResult::timed_out Fixes rust-lang#110045
…rkingjubilee Fix the example in document for WaitTimeoutResult::timed_out Fixes rust-lang#110045
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#110056 (Fix the example in document for WaitTimeoutResult::timed_out) - rust-lang#112655 (Mark `map_or` as `#[must_use]`) - rust-lang#114018 (Make `--error-format human-annotate-rs` handle multiple files) - rust-lang#114068 (inline format!() args up to and including rustc_middle (2)) - rust-lang#114223 (Documentation: Fix Stilted Language in Vec->Indexing) - rust-lang#114227 (Add tidy check for stray rustfix files) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #110045