Skip to content
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

Improve documentation on process::Child.std* fields #74192

Merged
merged 1 commit into from
Aug 15, 2020

Conversation

xkr47
Copy link
Contributor

@xkr47 xkr47 commented Jul 9, 2020

As a relative beginner, it took a while for me to figure out I could just steal the references to avoid partially moving the child and thus retain ability to call functions on it (and store it in structs etc).

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 9, 2020
/// ```
/// let stdin = child.stdin.take().unwrap();
/// ```
/// to avoid partially moving the `child` and thus block yourself from calling
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// to avoid partially moving the `child` and thus block yourself from calling
/// to avoid partially moving the `child` and thus blocking yourself from calling

src/libstd/process.rs Outdated Show resolved Hide resolved
src/libstd/process.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Jul 28, 2020

☔ The latest upstream changes (presumably #73265) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnTitor JohnTitor added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2020
src/libstd/process.rs Outdated Show resolved Hide resolved
@JohnTitor
Copy link
Member

This has no response from the reviewer for a while.
I think we could move these comments into the struct's comment to avoid repitition but I'm going to defer to r? @Mark-Simulacrum anyway.

@Mark-Simulacrum
Copy link
Member

@bors r+

Applied suggestions and squashed. Thanks!

@bors
Copy link
Contributor

bors commented Aug 14, 2020

📌 Commit a021b9219e0d2544374046289d9054947798231d has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 14, 2020
@lnicola
Copy link
Member

lnicola commented Aug 14, 2020

The doctests need to be marked with ignore (see) so that they aren't compiled or executed.

I'd also add a newline before the first fence. I'm not sure whether it affects the rendering, but it does seem to be the convention.

@JohnTitor
Copy link
Member

@bors r- rollup=always —^ and it’s docs change only.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 14, 2020
As a relative beginner, it took a while for me to figure out I could just steal the references to avoid partially moving the child and thus retain ability to call functions on it (and store it in structs etc).
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 14, 2020

📌 Commit 90e4c90 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 14, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2020
Rollup of 17 pull requests

Successful merges:

 - rust-lang#73943 (Document the unsafe keyword)
 - rust-lang#74062 (deny(unsafe_op_in_unsafe_fn) in libstd/ffi/c_str.rs)
 - rust-lang#74185 (Remove liballoc unneeded explicit link)
 - rust-lang#74192 (Improve documentation on process::Child.std* fields)
 - rust-lang#74409 (Change Debug impl of SocketAddr and IpAddr to match their Display output)
 - rust-lang#75195 (BTreeMap: purge innocent use of into_kv_mut)
 - rust-lang#75214 (Use intra-doc links in `mem::manually_drop` & `mem::maybe_uninit`)
 - rust-lang#75432 (Switch to intra-doc links in `std::process`)
 - rust-lang#75482 (Clean up E0752 explanation)
 - rust-lang#75501 (Move to intra doc links in std::ffi)
 - rust-lang#75509 (Tweak suggestion for `this` -> `self`)
 - rust-lang#75511 (Do not emit E0228 when it is implied by E0106)
 - rust-lang#75515 (Bump std's libc version to 0.2.74)
 - rust-lang#75517 (Promotion and const interning comments)
 - rust-lang#75519 (BTreeMap: refactor splitpoint and move testing over to unit test)
 - rust-lang#75530 (Switch to intra-doc links in os/raw/*.md)
 - rust-lang#75531 (Migrate unit tests of btree collections to their native breeding ground)

Failed merges:

r? @ghost
@bors bors merged commit dae020d into rust-lang:master Aug 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants