-
Notifications
You must be signed in to change notification settings - Fork 340
Add initial Fuse implementation for Stream #40
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
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!
Really, I think it's a very nasty usability problem that futures::select!
requires a FusedStream
(as opposed to calling .fuse
internally, like it is recommended to do for iterators), but I think I see why this needs to be the case.
I wonder if we have long-term plan here... Could we just require that all streams are fused?
Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
4c45443
to
aa94d45
Compare
Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
I've rebased @spacejam's excellent work on master; it's all passing now. Also removed the two helper methods on the Given the amount of review this has had, I think this should be good to merge. Ref #129. bors r+ |
Build succeeded
|
@matklad does this address your use case?