-
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
Add documentation for ExitStatus
#42024
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (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. |
/// | ||
/// This `struct` is used to represent the exit status of a child process. | ||
/// Child processes are created via the [`Command`] struct and their exit | ||
/// status is exposed through the [`status`] method. |
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 think these two (status
and Command
) are missing the URL portion.
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.
Yup! The
[`Command`]: URL
part
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.
@steveklabnik @Mark-Simulacrum I added them just now.
src/libstd/process.rs
Outdated
/// | ||
/// # Examples | ||
/// | ||
/// ```rust,no_run |
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.
The rust tag is useless (it's enabled by default).
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.
Removed it. I had actually copied this over from another example in the same file.
Yeah, no worries; some of the stuff is in an older style.
…On Tue, May 16, 2017 at 11:33 AM, Michael Kohl ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/libstd/process.rs
<#42024 (comment)>:
> @@ -788,6 +795,22 @@ impl ExitStatus {
/// On Unix, this will return `None` if the process was terminated
/// by a signal; `std::os::unix` provides an extension trait for
/// extracting the signal and other details from the `ExitStatus`.
+ ///
+ /// # Examples
+ ///
+ /// ```rust,no_run
Removed it. I had actually copied this over from another example in the
same file.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#42024 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABsihk15iNAUEWDjAboSMReHuNKGut9ks5r6cHAgaJpZM4Nb91Z>
.
|
src/libstd/process.rs
Outdated
/// Some(code) => println!("Exited with status code: {}", code), | ||
/// None => println!("Process terminated by signal") | ||
/// } | ||
/// ... |
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.
...
→ ```
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.
🤦♂️ fixed, thanks.
Needing several follow-up commits for a doc PR, not my proudest moment 😂 |
It's no big deal at all, it happens all the time :)
…On Tue, May 16, 2017 at 11:53 AM, Michael Kohl ***@***.***> wrote:
Needing several follow-up commits for a doc PR, not my proudest moment 😂
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#42024 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABsinFcg6wFmBeI4CEBxzh_K7pqPFdjks5r6cZ9gaJpZM4Nb91Z>
.
|
Two things now: tests seem to fail, so please fix them. :p Then please squash your commits. |
@GuillaumeGomez Yeah, I was about to squash the commits. Re the failing specs, I was wondering how a docs only commit would break anything, gotta look into that when I have some time. |
c225ee2
to
75ee366
Compare
I was wondering how a docs only commit would break anything
Code examples are run as tests!
…On Wed, May 17, 2017 at 4:19 AM, Michael Kohl ***@***.***> wrote:
@GuillaumeGomez <https://github.com/guillaumegomez> Yeah, I was about to
squash the commits. Re the failing specs, I was wondering how a docs only
commit would break anything, gotta look into that when I have some time.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#42024 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABsigoXmEavStyk_VSo7JuzIU4Rn2y8ks5r6q2SgaJpZM4Nb91Z>
.
|
In your case this is an invalid linkage:
|
As requested in rust-lang#29370.
75ee366
to
b2fc7b1
Compare
I see, doctests. Well, this commit will hopefully finally fix this issue. |
It did! 🎊 @bors: r+ rollup |
📌 Commit b2fc7b1 has been approved by |
…r=steveklabnik Add documentation for `ExitStatus` As requested in rust-lang#29370. r? @steveklabnik
…r=steveklabnik Add documentation for `ExitStatus` As requested in rust-lang#29370. r? @steveklabnik
…r=steveklabnik Add documentation for `ExitStatus` As requested in rust-lang#29370. r? @steveklabnik
⌛ Testing commit b2fc7b1 with merge 74ac2d2... |
💔 Test failed - status-appveyor |
…r=steveklabnik Add documentation for `ExitStatus` As requested in rust-lang#29370. r? @steveklabnik
As requested in #29370. r? @steveklabnik