Skip to content

[Doc] improve thread::Thread and thread::Builder documentations #41814

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

Merged
merged 3 commits into from
May 9, 2017

Conversation

gamazeps
Copy link
Contributor

@gamazeps gamazeps commented May 7, 2017

Part of #29378

  • Adds information about the stack_size when using Builder. This might be considered too low level, but I assume that if someone wants to create their own builder instead of using thread::spawn they may be interested in that info.
  • Updates the thread::Thread structure doc, mostly by explaining how to get one, the previous example was removed because it was not related to thread::Thread, but rather to thread::Builder::name.
    Not much is present there, mostly because this API is not often used (the only method that seems useful is unpark, which is documented in [DOC] Improve the thread::park and thread::unpark documentation #41809).

Felix Raimundo added 2 commits May 7, 2017 19:31
- Copied the module documentation to `Thread`.
- Removed the example because it did not use any method of Thread.
@rust-highfive
Copy link
Contributor

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@gamazeps
Copy link
Contributor Author

gamazeps commented May 7, 2017

r? @steveklabnik

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2017
Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

some small details! thank you!

@@ -244,6 +244,11 @@ impl Builder {
/// Generates the base configuration for spawning a thread, from which
/// configuration methods can be chained.
///
/// If the [`stack_size`][stack_size] field is not specified, the stack size
Copy link
Member

Choose a reason for hiding this comment

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

changing this to just have

[`stack_size`]

would follow convention

@@ -259,6 +264,8 @@ impl Builder {
///
/// handler.join().unwrap();
/// ```
///
/// [stack_size]: ../../std/thread/struct.Builder.html#method.stack_size
Copy link
Member

Choose a reason for hiding this comment

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

that means writing

[`stack_size`]: ../../std/

here

@@ -714,33 +721,21 @@ struct Inner {

#[derive(Clone)]
#[stable(feature = "rust1", since = "1.0.0")]
/// A handle to a thread.
/// Threads are represented via the `Thread` type, which you can get in one of
/// two ways:
Copy link
Member

Choose a reason for hiding this comment

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

This first line needs to be a summary; this doesn't quite work. I think leaving the first line, adding a /// in between, and keeping this new stuff is 👍

@gamazeps
Copy link
Contributor Author

gamazeps commented May 9, 2017

Done :)

@steveklabnik
Copy link
Member

@bors: r+ rollup

thanks a ton!

@bors
Copy link
Collaborator

bors commented May 9, 2017

📌 Commit 323a774 has been approved by steveklabnik

@bors
Copy link
Collaborator

bors commented May 9, 2017

⌛ Testing commit 323a774 with merge 644fc40...

bors added a commit that referenced this pull request May 9, 2017
[Doc] improve `thread::Thread` and `thread::Builder` documentations

Part of #29378

- Adds information about the stack_size when using `Builder`. This might be considered too low level, but I assume that if someone wants to create their own builder instead of using `thread::spawn` they may be interested in that info.
- Updates the `thread::Thread` structure doc, mostly by explaining how to get one, the previous example was removed because it was not related to `thread::Thread`, but rather to `thread::Builder::name`.
  Not much is present there, mostly because this API is not often used (the only method that seems useful is `unpark`, which is documented in #41809).
@bors
Copy link
Collaborator

bors commented May 9, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: steveklabnik
Pushing 644fc40 to master...

@bors bors merged commit 323a774 into rust-lang:master May 9, 2017
@gamazeps gamazeps deleted the thread-struct-doc branch May 13, 2017 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants