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

Don't encourage people to ignore threading errors in the docs #44962

Merged
merged 1 commit into from Oct 10, 2017

Conversation

@shepmaster
Member

shepmaster commented Oct 1, 2017

No description provided.

@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Oct 1, 2017

Collaborator

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

Collaborator

rust-highfive commented Oct 1, 2017

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

Show outdated Hide outdated src/libstd/thread/mod.rs Outdated
@sfackler

This comment has been minimized.

Show comment
Hide comment
@sfackler
Member

sfackler commented Oct 1, 2017

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Oct 2, 2017

Member

I think it's all good except for the issue you pointed at. Once done, it's all good for me.

Member

GuillaumeGomez commented Oct 2, 2017

I think it's all good except for the issue you pointed at. Once done, it's all good for me.

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Oct 5, 2017

Member

@bors: r+ rollup

thanks!

Member

steveklabnik commented Oct 5, 2017

@bors: r+ rollup

thanks!

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 5, 2017

Contributor

📌 Commit 928013b has been approved by steveklabnik

Contributor

bors commented Oct 5, 2017

📌 Commit 928013b has been approved by steveklabnik

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Oct 6, 2017

Member

Sorry @steveklabnik, I just saw a lint so I'll r- for now. ;)

@bors: r-

Member

GuillaumeGomez commented Oct 6, 2017

Sorry @steveklabnik, I just saw a lint so I'll r- for now. ;)

@bors: r-

@@ -485,15 +485,17 @@ impl Builder {
/// let (tx, rx) = channel();
///
/// let sender = thread::spawn(move || {
/// let _ = tx.send("Hello, thread".to_owned());
/// tx.send("Hello, thread".to_owned())
/// .expect("Unable to send on channel");

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Oct 6, 2017

Member

Invalid indent, should be:

tx.send("Hello, thread".to_owned())
  .expect("Unable to send on channel");
@GuillaumeGomez

GuillaumeGomez Oct 6, 2017

Member

Invalid indent, should be:

tx.send("Hello, thread".to_owned())
  .expect("Unable to send on channel");

This comment has been minimized.

@shepmaster

shepmaster Oct 6, 2017

Member

Invalid indent

Huh. Rustfmt is what indented it this way. Where is this style documented so I can check it before submitting future PRs?

@shepmaster

shepmaster Oct 6, 2017

Member

Invalid indent

Huh. Rustfmt is what indented it this way. Where is this style documented so I can check it before submitting future PRs?

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Oct 7, 2017

Member

rustfmt is really bad... Well, whatever, please align the methods' calls.

@GuillaumeGomez

GuillaumeGomez Oct 7, 2017

Member

rustfmt is really bad... Well, whatever, please align the methods' calls.

This comment has been minimized.

@shepmaster

shepmaster Oct 7, 2017

Member

Where is this style documented so I can check it before submitting future PRs?

@shepmaster

shepmaster Oct 7, 2017

Member

Where is this style documented so I can check it before submitting future PRs?

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Oct 8, 2017

Member

No clue. It's how it's done in the rest of rustc source code. Sorry but I have no idea if there is such a document...

@GuillaumeGomez

GuillaumeGomez Oct 8, 2017

Member

No clue. It's how it's done in the rest of rustc source code. Sorry but I have no idea if there is such a document...

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 8, 2017

Contributor

💡 This pull request was already approved, no need to approve it again.

  • There's another pull request that is currently being tested, blocking this pull request: #45016
Contributor

bors commented Oct 8, 2017

💡 This pull request was already approved, no need to approve it again.

  • There's another pull request that is currently being tested, blocking this pull request: #45016
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 8, 2017

Contributor

📌 Commit 37724f8 has been approved by steveklabnik

Contributor

bors commented Oct 8, 2017

📌 Commit 37724f8 has been approved by steveklabnik

@rust-lang rust-lang deleted a comment from bors Oct 8, 2017

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Oct 8, 2017

Member

@bors: r+

This is in fact standard style, which rustc has not yet fully adopted, but will. We want to show standard style in the docs. I can understand if you don't like that style, @GuillaumeGomez , as there's some parts I'm not psyched about either, but sticking with the standard is worth arguing over quibbles like these.

Member

steveklabnik commented Oct 8, 2017

@bors: r+

This is in fact standard style, which rustc has not yet fully adopted, but will. We want to show standard style in the docs. I can understand if you don't like that style, @GuillaumeGomez , as there's some parts I'm not psyched about either, but sticking with the standard is worth arguing over quibbles like these.

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 8, 2017

Contributor

📌 Commit b5b7666 has been approved by steveklabnik

Contributor

bors commented Oct 8, 2017

📌 Commit b5b7666 has been approved by steveklabnik

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Oct 9, 2017

Member

My bad, thought it was the standard style.

Member

GuillaumeGomez commented Oct 9, 2017

My bad, thought it was the standard style.

kennytm added a commit to kennytm/rust that referenced this pull request Oct 9, 2017

Rollup merge of rust-lang#44962 - shepmaster:no-ignore-result, r=stev…
…eklabnik

Don't encourage people to ignore threading errors in the docs

@kennytm kennytm referenced this pull request Oct 9, 2017

Merged

Rollup of 9 pull requests #45141

kennytm added a commit to kennytm/rust that referenced this pull request Oct 9, 2017

Rollup merge of rust-lang#44962 - shepmaster:no-ignore-result, r=stev…
…eklabnik

Don't encourage people to ignore threading errors in the docs

bors added a commit that referenced this pull request Oct 9, 2017

Auto merge of #45141 - kennytm:rollup, r=kennytm
Rollup of 9 pull requests

- Successful merges: #44962, #45051, #45091, #45106, #45117, #45118, #45120, #45125, #45136
- Failed merges:

bors added a commit that referenced this pull request Oct 10, 2017

Auto merge of #45141 - kennytm:rollup, r=kennytm
Rollup of 9 pull requests

- Successful merges: #44962, #45051, #45091, #45106, #45117, #45118, #45120, #45125, #45136
- Failed merges:

@bors bors merged commit b5b7666 into rust-lang:master Oct 10, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@shepmaster shepmaster deleted the shepmaster:no-ignore-result branch Oct 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment