-
Notifications
You must be signed in to change notification settings - Fork 13.3k
doc: nits and fixes for thread API #22689
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
(rust_highfive has picked a reviewer for you, use r? to override) |
//! | ||
//! ## The `Thread` type | ||
//! | ||
//! Already-running threads are represented via the `Thread` type, which you can | ||
//! Threads are represented via the `Thread` type, which you can |
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.
Thread
still only represents already-running threads.
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.
I removed Already-running because it doesn't work well with the line that follows it:
By spawning a new thread, e.g. using the
thread::spawn
function.
//! panic may optionally be detected from a different thread. If | ||
//! the main thread panics the application will exit with a non-zero | ||
//! the panic may optionally be detected from a different thread. If | ||
//! the main thread panics, the application will exit with a non-zero |
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.
I don't think this comma is necessary
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.
If the comma is removed, the sentence will have to be slightly re-organised by starting with "The application...". Otherwise, the pause feels very needed.
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.
@alexcrichton The comma usage here is correct. See https://owl.english.purdue.edu/owl/resource/607/02/ item 2.
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.
Some people leave it in, some leave it out. I generally put it in. It's fine either way.
I may not be the best wordsmith |
wow, that felt long; thanks @steveklabnik |
No description provided.