search

smol-rs / futures-lite

Futures, streams, and async I/O combinators.
Apache License 2.0
449 stars 26 forks source link

Rename try_stream to drain #96

Closed notgull closed 6 months ago

notgull commented 7 months ago

At the suggestion of @i509vcb

taiki-e commented 7 months ago

Thanks. I think this is much clearer than the previous one, but I would like to hear @smol-rs/admins's opinions.

zeenix commented 6 months ago

Thanks. I think this is much clearer than the previous one

Although I agree but seems the current name was chosen for consistency with std:

/// This is intended to be used as a way of polling a stream without waiting, similar to the /// [try_iter] function on [std::sync::mpsc::Receiver]. For instance, running this stream /// on an [async_channel::Receiver] will return all messages that are currently in the /// channel, but will not wait for new messages.

zeenix commented 6 months ago

If we do this, we should probably do it in a backwards-compatible way using type aliases.

taiki-e commented 6 months ago

If we do this, we should probably do it in a backwards-compatible way using type aliases.

That is not necessary because the old name has not been released. (See https://github.com/smol-rs/futures-lite/pull/94 for the context of this PR.)

notgull commented 6 months ago

/// This is intended to be used as a way of polling a stream without waiting, similar to the /// [try_iter] function on [std::sync::mpsc::Receiver]. For instance, running this stream /// on an [async_channel::Receiver] will return all messages that are currently in the /// channel, but will not wait for new messages.

Unfortunately it's not really an Iterator so we can't name it that.

zeenix commented 6 months ago

Unfortunately it's not really an Iterator so we can't name it that.

I wasn't suggesting we name it an iterator. That was quoting the existing docs, which justify the try_stream name for consistency with std.

notgull commented 6 months ago

I wasn't suggesting we name it an iterator. That was quoting the existing docs, which justify the try_stream name for consistency with std.

The issue with that is that TryStream is already a taken name in futures, and I think naming the combinator this would cause confusion. Hence the change to Drain.

fogti commented 6 months ago

how does this compare to futures' TryStream then?

notgull commented 6 months ago

how does this compare to futures' TryStream then?

Mostly unrelated. TryStream is a Stream that returns a Result, while Drain is a Stream combinator that returns as many elements that can be returned as possible without waiting.

fogti commented 6 months ago

ok, then I side with you that try_stream is unfortunate naming.