-
Notifications
You must be signed in to change notification settings - Fork 341
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 Stream::scan #192
Add Stream::scan #192
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.
Thank you, this looks great! I just have a small request for changes around Unpin
.
where | ||
Self: Sized, | ||
St: Unpin, | ||
F: Unpin + FnMut(&mut St, Self::Item) -> Option<B>, |
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.
We don't need St
nor F
to be Unpin
because we're never pinning them.
Thanks! bors r+ |
192: Add Stream::scan r=stjepang a=tirr-c Ref #129. The mapper function `f` is synchronous and returns bare `Option<B>`. Asynchronous `f` seems tricky to implement right. It requires the wrapper to be self-referential, as a reference to internal state may be captured by the returned future. Co-authored-by: Wonwoo Choi <[email protected]>
Build succeeded
|
196: Remove more Unpin bounds for Stream::scan r=stjepang a=tirr-c cc #192. I missed the bounds on `Stream::scan` itself. Co-authored-by: Wonwoo Choi <[email protected]>
Ref #129. The mapper function
f
is synchronous and returns bareOption<B>
.Asynchronous
f
seems tricky to implement right. It requires the wrapper to be self-referential, as a reference to internal state may be captured by the returned future.