Uh oh!
There was an error while loading. Please reload this page.
Update ChildStderr docs to be clearer - #40829
Conversation
Before the docs only had a line about where it was found and that it was a handle to stderr. This commit changes it so that the summary second line is removed and that it's a bit clearer about what can be done with it. Part of \#29370
mgattozzi
commented
Mar 25, 2017
rust-highfive
commented
Mar 25, 2017
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @BurntSushi (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
mgattozzi
commented
Mar 25, 2017
@steveklabnik I'm not sure if this is what you meant by expanding upon the docs in #29370. If it is great because I have two PRs ready to go for ChildStdin and ChildStdout, but I figured it best to get a sense for what you wanted this to be expanded too here before opening those two. |
| /// A handle to a child process's stderr. This struct is used in the [`stderr`] | ||
| /// field on [`Child`]. | ||
| /// A handle to a child process's stderr. It can be used to |
There was a problem hiding this comment.
Please keep "This struct is used in the [stderr] field on [Child]."
There was a problem hiding this comment.
I'm not saying to remove it, I'm saying that there needs to be whitespace between.
There was a problem hiding this comment.
Okay will do. That makes more sense.
| /// A handle to a child process's stderr. This struct is used in the [`stderr`] | ||
| /// field on [`Child`]. | ||
| /// A handle to a child process's stderr. It can be used to |
There was a problem hiding this comment.
So what we need is a newline between this sentence and the next. rustdoc treats the first paragraph as the summary so
/// A handle to a child process' stderr.
///
/// It can be used to....
that kind of thing. make sense?
There was a problem hiding this comment.
I'd prefer to have it this way as well.
| /// A handle to a child process's stderr. This struct is used in the [`stderr`] | ||
| /// field on [`Child`]. | ||
| /// A handle to a child process's stderr. It can be used to |
There was a problem hiding this comment.
I'm not saying to remove it, I'm saying that there needs to be whitespace between.
steveklabnik
commented
Mar 27, 2017
wow thanks github, dunno how that got duplicated... |
mgattozzi
commented
Mar 29, 2017
I see what your wording in the issue means now. After my other PR is done building locally I can move on to this one. |
mgattozzi
commented
Apr 4, 2017
@GuillaumeGomez and @steveklabnik I've updated the docs to have the newlines and changed it back to the original wording as requested! |
steveklabnik
commented
Apr 9, 2017
@bors: r+ rollup |
bors
commented
Apr 9, 2017
📌 Commit c7f2dbe has been approved by |
bors
commented
Apr 9, 2017
Update ChildStderr docs to be clearer Before the docs only had a line about where it was found and that it was a handle to stderr. This commit changes it so that the summary second line is removed and that it's a bit clearer about what can be done with it. Part of #29370
bors
commented
Apr 9, 2017
☀️ Test successful - status-appveyor, status-travis |
mgattozzi
commented
Apr 9, 2017
Awesome! I'll get the other two in soon so that we can knock them off the process list. |
…eveklabnik Update ChildStdin/ChildStdout docs to be clearer This is part of rust-lang#29370 and continues the work from rust-lang#40829
…eveklabnik Update ChildStdin/ChildStdout docs to be clearer This is part of rust-lang#29370 and continues the work from rust-lang#40829
Before the docs only had a line about where it was found and that it was
a handle to stderr. This commit changes it so that the summary second line is
removed and that it's a bit clearer about what can be done with it. Part of
#29370