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 and fix mpsc documentation #37941

Merged
merged 1 commit into from
Dec 13, 2016
Merged

Conversation

Cobrand
Copy link
Contributor

@Cobrand Cobrand commented Nov 22, 2016

Closes #37915

This commit enhances documentation with several links and
fixes an error in the sync_channel documentation as well:
send doesn't panic when the senders are all disconnected

r? @steveklabnik

@mfarrugi
Copy link

Are the references to 'runtime' also outdated? eg. L235

@Cobrand
Copy link
Contributor Author

Cobrand commented Nov 25, 2016

@mfarrugi I don't really get what you mean ... could you explain more in detail ?

@mfarrugi
Copy link

I'm not clear on what runtime refers to in
// A major goal of these channels is to work seamlessly on and off the runtime.
But I think it refers to the old rust runtime

@Cobrand
Copy link
Contributor Author

Cobrand commented Nov 25, 2016

Indeed it looks like the mention of the runtime is part of the old documentation that was kept here.

A quick git blame told me that most the the comments in this file were from 2013 or 2014, so definitely way before the first release of Rust.

That might need another PR though ?

@bors
Copy link
Contributor

bors commented Nov 26, 2016

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

@Cobrand
Copy link
Contributor Author

Cobrand commented Dec 6, 2016

This has been opened for several days but I don't see any issue, I guess I'll try my luck with someone else ?
r? @GuillaumeGomez

@@ -458,6 +458,13 @@ impl<T> UnsafeFlavor<T> for Receiver<T> {
/// All data sent on the sender will become available on the receiver, and no
/// send will block the calling thread (this channel has an "infinite buffer").
///
/// If the [`Receiver`] is disconnected while trying to [`send`] with the
Copy link
Member

Choose a reason for hiding this comment

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

All functions/methods name should have () at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

/// becomes available. These channels differ greatly in the semantics of the
/// sender from asynchronous channels, however.
///
/// This channel has an internal buffer on which messages will be queued. `bound`
/// specifies the buffer size. When the internal buffer becomes full, future sends
/// will *block* waiting for the buffer to open up. Note that a buffer size of 0
/// is valid, in which case this becomes "rendezvous channel" where each send will
/// not return until a recv is paired with it.
/// is valid, in which case this becomes "rendezvous channel" where each [`send`]
Copy link
Member

Choose a reason for hiding this comment

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

Extra whitespace before "rendezvous". By the way, shouldn't it be rendez-vous? French in english is so strange haha.

Copy link
Member

Choose a reason for hiding this comment

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

Oh and same comment for send in here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Being french myself I had my doubts about that but in English it's definitely rendezvous :

https://fr.wikipedia.org/wiki/Rendez-vous
https://en.wikipedia.org/wiki/Rendezvous

For some unknown and I guess historical reasons the translators dropped the dash

/// will not return until a recv is paired with it.
///
/// Like asynchronous channels, if the [`Receiver`] is disconnected while trying
/// to [`send`] with the [`SyncSender`], the [`send`] method will return an
Copy link
Member

Choose a reason for hiding this comment

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

Same comment for send.

Closes rust-lang#37915

This commit enhances documentation with several links and
fixes an error in the `sync_channel` documentation as well:
`send` doesn't panic when the senders are all disconnected
@Cobrand Cobrand force-pushed the docfix-issue-37915 branch from b4db1a7 to 57f998a Compare December 7, 2016 17:57
@GuillaumeGomez
Copy link
Member

All good, thanks!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Dec 7, 2016

📌 Commit 57f998a has been approved by GuillaumeGomez

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Dec 12, 2016
…aumeGomez

Improve and fix mpsc documentation

Closes rust-lang#37915

This commit enhances documentation with several links and
fixes an error in the `sync_channel` documentation as well:
`send` doesn't panic when the senders are all disconnected

r? @steveklabnik
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Dec 12, 2016
…aumeGomez

Improve and fix mpsc documentation

Closes rust-lang#37915

This commit enhances documentation with several links and
fixes an error in the `sync_channel` documentation as well:
`send` doesn't panic when the senders are all disconnected

r? @steveklabnik
bors added a commit that referenced this pull request Dec 13, 2016
Rollup of 7 pull requests

- Successful merges: #37052, #37941, #38067, #38164, #38202, #38264, #38299
- Failed merges:
@bors bors merged commit 57f998a into rust-lang:master Dec 13, 2016
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.

Documentation phrase about panics in mpsc channel may be obsolete
5 participants