Channel error docs #41103
Channel error docs #41103
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. |
src/libstd/sync/mpsc/mod.rs
Outdated
| #[derive(PartialEq, Eq, Clone, Copy, Debug)] | ||
| #[stable(feature = "rust1", since = "1.0.0")] | ||
| pub enum TryRecvError { | ||
| /// This channel is currently empty, but the sender(s) have not yet | ||
| /// This `channel` is currently empty, but the `Sender`(s) have not yet |
There was a problem hiding this comment.
channel and Sender here are ambiguous, could refer to either sync or async. How should something like this be resolved?
There was a problem hiding this comment.
By providing a little extra explanation. I think it's the easiest way.
There was a problem hiding this comment.
I added a bit of extra information + links above.
src/libstd/sync/mpsc/mod.rs
Outdated
| #[derive(PartialEq, Eq, Clone, Copy, Debug)] | ||
| #[stable(feature = "mpsc_recv_timeout", since = "1.12.0")] | ||
| pub enum RecvTimeoutError { | ||
| /// This channel is currently empty, but the sender(s) have not yet | ||
| /// This `channel` is currently empty, but the `Sender`(s) have not yet |
| @@ -544,36 +551,46 @@ impl<T> UnsafeFlavor<T> for Receiver<T> { | |||
| } | |||
|
|
|||
| /// Creates a new asynchronous channel, returning the sender/receiver halves. | |||
| /// 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"). | |||
| /// All data sent on the [`Sender`] will become available on the [`Receiver`] in | |||
There was a problem hiding this comment.
Expanding the docs on channel (and sync_channel below) - most of this is adapted from the mspc module page since that's where the bulk of this is explained.
|
I will review this tomorrow, unless someone else from @rust-lang/docs beats me to it 😄 |
|
@projektir: Actually, except for the confusing things you reported, it's all good. Please just add a small explanation every time it's needed and it'll be all good. Also, when done updating, please squash your commits. |
src/libstd/sync/mpsc/mod.rs
Outdated
| /// This channel's sending half has become disconnected, and there will | ||
| /// never be any more data received on this channel | ||
| /// This `channel`'s sending half has become disconnected, and there will | ||
| /// never be any more data received on this channel. |
There was a problem hiding this comment.
channel is repeated here. Is it wanted?
There was a problem hiding this comment.
I don't really have a strong opinion on it.
There was a problem hiding this comment.
@GuillaumeGomez are you looking for a change here?
There was a problem hiding this comment.
I don't see anything wrong with mentioning 'channel' twice here.
There was a problem hiding this comment.
@frewsxcv yeah, this was the original text and it sounded OK to me so I left it alone, but after talking to @GuillaumeGomez I changed it to a more terse sentence (check the latest commit, this is outdated).
There was a problem hiding this comment.
We tend to use backticks (`) to form code-related words, not bold.
There was a problem hiding this comment.
@frewsxcv ...then you and @GuillaumeGomez may be in disagreement. I was using (`) prior to mark channel and Sender, but I was not linking them because they are ambiguous (could be sync_channel and SyncSender. We talked about it and opted for bold instead.
So we need to figure out how this should be resolved long term, because I've already updated my PR like 3 times and I'm rather confused as to what's expected here. 😛
There was a problem hiding this comment.
Oh I understand, nevermind my comment. Sorry for the confusion!
I'll let @GuillaumeGomez review since it sounds like you've already spoken with them about this pull request.
…cumentation to channel() and sync_channel(); adding more links #29377
|
@GuillaumeGomez I added bolding instead of the `. I think it works well. Commits are squashed. |
|
Thanks! @bors: r+ rollup |
|
📌 Commit 28a232a has been approved by |
Channel error docs r? @steveklabnik I'm going to need some help on this one, a few ambiguities.
|
☀️ Test successful - status-appveyor, status-travis |
r? @steveklabnik
I'm going to need some help on this one, a few ambiguities.