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

stream: expose Elapsed #4502

Merged
merged 2 commits into from
Mar 2, 2022
Merged

Conversation

lambdalisue
Copy link
Contributor

Motivation

We cannot use an error on StreamExt::timeout()

Solution

Expose Elapsed as tokio_stream::Elapsed

Note

It seems #4286 points the same issue but it looked like it had stopped while so I made a new one.

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

This looks good to me, I'm pretty sure not exporting that error type was an oversight and it was intended to be public.

One minor note is that we may want to update the RustDoc for Elapsed to actually link to the StreamExt::timeout method:

/// Error returned by `Timeout`.

@Darksonn Darksonn added A-tokio-stream Area: The tokio-stream crate M-time Module: tokio/time labels Feb 16, 2022
@lambdalisue
Copy link
Contributor Author

Is there anything need to complete?

Comment on lines +80 to +82
cfg_time! {
pub use stream_ext::timeout::Elapsed;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

My main concern is whether this should be in a dedicated module or not.

@Darksonn Darksonn merged commit 014be71 into tokio-rs:master Mar 2, 2022
@lambdalisue lambdalisue deleted the stream-expose-errors branch March 2, 2022 15:00
@lambdalisue
Copy link
Contributor Author

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-stream Area: The tokio-stream crate M-time Module: tokio/time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants