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

Various std::process doc improvements. #34737

Merged
merged 1 commit into from Jul 12, 2016

Conversation

frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Jul 9, 2016

No description provided.

@@ -91,7 +96,11 @@ impl IntoInner<imp::Process> for Child {
fn into_inner(self) -> imp::Process { self.handle }
}

/// A handle to a child process's stdin
/// A handle to a child process's stdin. This struct is used in the [`stdin`]
Copy link
Member

Choose a reason for hiding this comment

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

I'm not an english native but I'm pretty sure you don't write "process's" but "process'". But maybe I'm wrong so please correct me if this is the case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would personally say process's, but I might be wrong and English is weird.

Copy link
Member

Choose a reason for hiding this comment

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

Ah. It doesn't help. 😆

Copy link
Contributor

Choose a reason for hiding this comment

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

process's is correct.

Copy link
Member

Choose a reason for hiding this comment

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

Then my bad!

@GuillaumeGomez
Copy link
Member

For such small changes, I think one commit is more than enough.

@frewsxcv
Copy link
Member Author

Squashed.

@GuillaumeGomez
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jul 10, 2016

📌 Commit d0b8114 has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented Jul 10, 2016

⌛ Testing commit d0b8114 with merge 17964c8...

@bors
Copy link
Contributor

bors commented Jul 10, 2016

💔 Test failed - auto-win-gnu-32-opt

@frewsxcv
Copy link
Member Author

Error looks unrelated?

@GuillaumeGomez
Copy link
Member

Strange...

@bors: retry

@bors
Copy link
Contributor

bors commented Jul 10, 2016

⌛ Testing commit d0b8114 with merge e6576bc...

@bors
Copy link
Contributor

bors commented Jul 11, 2016

💔 Test failed - auto-win-gnu-32-opt-rustbuild

@frewsxcv
Copy link
Member Author

Linkcheck stage2 (i686-pc-windows-gnu)
std\process\struct.Child.html:95: broken link - std\process\or other functions that wrap around it
thread 'main' panicked at 'found some broken links', C:\bot\slave\auto-win-gnu-32-opt-rustbuild\build\src\tools\linkchecker\main.rs:54
note: Run with `RUST_BACKTRACE=1` for a backtrace.

If anyone has any idea which of my links is broken, let me know. I don't have enough time right now to look into it, but it's not obvious to me which is broken.

@steveklabnik
Copy link
Member

It's not clear to me either :/

///
/// Calling `wait` (or other functions that wrap around it) will make the
/// Calling [`wait`] (or other functions that wrap around it) will make the
Copy link
Member

Choose a reason for hiding this comment

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

It is this link which is broken.

Copy link
Member Author

Choose a reason for hiding this comment

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

How is this link broken?

Copy link
Member Author

Choose a reason for hiding this comment

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

Does it think (or other functions that wrap around it) is the href? Is there a way around this if so?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that's how markdown is interpreting it, you may be able to escape it via \( I believe

Copy link
Member Author

Choose a reason for hiding this comment

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

I just force pushed. Changed it to:

[`wait`][`wait`]

* Link to `process::Command` from `process::Child`.
* Move out inline Markdown link in doc comment.
* Link to `process::Child::wait` from `process::Child`.
* Link to `process::Child` from `process::ChildStdin`.
* Link to `process::Child` from `process::ChildStdout`.
* Link to `process::Child` from `process::ChildStderr`.
@GuillaumeGomez
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jul 11, 2016

📌 Commit 97d96bd has been approved by GuillaumeGomez

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 12, 2016
…illaumeGomez

Various `std::process` doc improvements.

None
bors added a commit that referenced this pull request Jul 12, 2016
Rollup of 7 pull requests

- Successful merges: #34736, #34737, #34740, #34742, #34749, #34750, #34770
- Failed merges: #33951
@bors bors merged commit 97d96bd into rust-lang:master Jul 12, 2016
@frewsxcv frewsxcv deleted the libstd-process-child branch July 12, 2016 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants