Skip to content

Extend compatibility to Stream and Sink #1162

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

Merged
merged 6 commits into from
Aug 2, 2018

Conversation

Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Aug 2, 2018

Sink is unidirectional because Sink01::start_send was split into Sink03::poll_ready + Sink03::start_send which makes tracking the context across the two method calls painful (might be possible with a custom Compat struct for this case, but I didn't investigate much).

@@ -248,4 +254,14 @@ pub trait SinkExt: Sink {
{
Either::Right(self)
}

/// Wraps a [`Sink`] into a sink compatible with libraries using
/// futures 0.1 `Sink`. Requires the `compat` feature to enable.
Copy link
Contributor

Choose a reason for hiding this comment

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

to be enabled

@MajorBreakfast
Copy link
Contributor

Some lines are longer than 80 chars

/// Converts a futures 0.3 `TryFuture` into a futures 0.1 `Future`
/// and vice versa.
/// Converts a futures 0.3 `TryFuture`, `TryStream` or `Sink` into a futures 0.1
/// `Future` and vice versa.
Copy link
Contributor

Choose a reason for hiding this comment

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

These should be links

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah never mind. Let's do the docs stuff in another PR

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I started doing it then decided to just leave it for another pass.

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 2, 2018

Oh, are we using an 80 character limit? I just applied rustfmt cause there was a lot of copy-paste + renaming and I couldn't be bothered formatting manually. I can add a .rustfmt.toml and reduce the limit in that case.

@MajorBreakfast
Copy link
Contributor

I can add a .rustfmt.toml limit it in that case.

@Nemo157 That'd be great!

@cramertj
Copy link
Member

cramertj commented Aug 2, 2018

RE rustfmt: be aware that for async blocks and functions you'll need a recent version that includes rust-lang/rustfmt#2874

@MajorBreakfast MajorBreakfast merged commit 37fb821 into rust-lang:master Aug 2, 2018
@Nemo157
Copy link
Member Author

Nemo157 commented Aug 2, 2018

I was only applying it to a couple of files so luckily didn’t hit that, did discover rust-lang/rustfmt#2887 on my next change though 😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants