-
Notifications
You must be signed in to change notification settings - Fork 26
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 poll_recv method to Receiver #56
Conversation
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.
LGTM otherwise.
I added an example to @zeenix what do you reckon? :)
I wasn't sure what you meant here offhand (I can't see any such sections in the repo) |
Awesome!
Is it worth it? Do we envision any other information going in it other than the count? If not, I'd rather keep things simple and just return the integer. If we need an error type, then we just go with option#1.
I meant adding a |
My arguments for keeping
But happy to go either way if you'd prefer otherwise
There are no other Error sections in the library for other functions that return |
It's clear enough since it's the Err variant of a Result. :) The docs would remove any confusion (if there is still some). :)
usize also implements
I didn't realize that but just because we've been lazy so far, doesn't mean we should continue to be so. :) Also in other fallible methods, all variants of the returned result type are possible so it's not as important as when returning an Error and not all variants could be possible (which would be the case for By "convention", I meant the general conversion in the Rust world (I think it's even recommended in the book).
Yes please. |
Done and done! |
You're too efficient. Do it again! 😆 |
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.
Almost done. :)
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.
👍
@jsdw now if you could just update the PR description, I can squash merge with that as the commit message. 🙏 |
@zeenix good to go? :) |
This PR Adds a
poll_recv()
method to theReceiver
type. It returns the sameResult<T,RecvError>
type that thereceiver.recv()
future returns (hence the name).This method can be used when defining custom streams that internally make use of
async_broadcast
and want to know about whether theasync_broadcast
stream has overflowed or not.