-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Update Child
docs to not have a note section
#40812
Conversation
In #29370 it's noted that for "the Note shouldn't be one, and should come before the examples." This commit changes the positioning of the section and removes wording that said take note in order for it to flow better with the surrounding text and it's new position.
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. |
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.
A very small nit, and then this is good to go. Thanks a ton!
src/libstd/process.rs
Outdated
/// run, even after the `Child` handle to the child process has gone out of | ||
/// scope. | ||
/// | ||
/// Calling [`wait`][`wait`] (or other functions that wrap around it) will make |
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.
You don't need to duplicate [wait
] here, you can drop it, just like [Drop
] above.
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.
This was already here when I moved it so I had just kept it. I'll drop it.
@bors: r+ rollup thanks! I totally hear you about keeping it the same, might as well fix it while you're here 😄 |
📌 Commit 7ef1b39 has been approved by |
Hahaha yeah I'm not opposed to fixing it, I just wasn't sure if that was some weird markdown thing from before. I figured it'd be better not to touch it. |
src/libstd/process.rs
Outdated
/// run, even after the `Child` handle to the child process has gone out of | ||
/// scope. | ||
/// | ||
/// Calling [`wait`] (or other functions that wrap around it) will make |
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'm not sure it'll work. Better write: "[wait
][ wait
]"
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.
Ah, yes, the space. @mgattozzi did you happen to build this locally? It should work, but it might not.
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.
Yeah not a problem. Sorry was a bit busy this week let me compile locally
While waiting for others' confirmation, I'll put this PR on wait. Sorry for the bothering. @bors: r- |
This did break the link. Working on the fix now. |
@mgattozzi let me know when you have this one updated! |
[ wait ][ wait ] didn't work either. I think we just need to revert the changes. Thoughts @steveklabnik? |
That's... odd. Given that was the way it was defined below. |
Hmmm. Looking at it again I had it as "[ wait ][ wait ]" based off the above comments from @GuillaumeGomez let me retry it without the "" and see if that does it. |
Yes sure. We're not in a hurry. |
Yeah that definitely didn't do it. It worked as was before the change I think it might be best if we leave it that way. |
Should work just fine if we're following common mark spec properly. |
@nagisa: Indeed. |
Alright I'll do that then and see if it works out properly |
Friendly ping to keep this on your radar @mgattozzi! |
@bors: r+ rollup thank you! |
📌 Commit df383b9 has been approved by |
Update `Child` docs to not have a note section In rust-lang#29370 it's noted that for "the Note shouldn't be one, and should come before the examples." This commit changes the positioning of the section and removes wording that said take note in order for it to flow better with the surrounding text and it's new position.
Thanks @carols10cents for the ping. I was a bit busy recently so it was good to get this in. |
In #29370 it's noted that for "the Note shouldn't be one, and should come before
the examples." This commit changes the positioning of the section and removes
wording that said take note in order for it to flow better with the surrounding
text and it's new position.