Skip to content

Relax queue-related signatures to use QueueSink / QueueSource#2466

Merged
mpilquist merged 2 commits intotypelevel:mainfrom
armanbilge:relax-queue
Jul 8, 2021
Merged

Relax queue-related signatures to use QueueSink / QueueSource#2466
mpilquist merged 2 commits intotypelevel:mainfrom
armanbilge:relax-queue

Conversation

@armanbilge
Copy link
Copy Markdown
Member

@armanbilge armanbilge commented Jul 5, 2021 •
edited
Loading

Supersedes #2465

(GH wouldn't let me re-open the old PR)

@arosien
Copy link
Copy Markdown
Contributor

arosien commented Jul 5, 2021

Fixes #2437 I believe.

@mpilquist mpilquist merged commit ab3221d into typelevel:main Jul 8, 2021
@kubukoz
Copy link
Copy Markdown
Member

kubukoz commented Jul 12, 2021 •
edited
Loading

I'm just curious... I thought type erasure would consider these identical: okay I see, Queue and QueueSource aren't generics themselves so they'll end up as Queue and QueueSource in the bytecode. Obviously 😂

fs2/core/shared/src/main/scala/fs2/Stream.scala

Lines 3227 to 3245 in f855bf8

def fromQueueUnterminated[F[_]: Functor, A](
queue: QueueSource[F, A],
limit: Int = Int.MaxValue
): Stream[F, A] =
fromQueueNoneTerminatedChunk_[F, A](
queue.take.map(a => Some(Chunk.singleton(a))),
queue.tryTake.map(_.map(a => Some(Chunk.singleton(a)))),
limit
)
/** Returns a stream of elements from the supplied queue.
*
* All elements that are available, up to the specified limit,
* are dequeued and emitted as a single chunk.
*/
def fromQueueUnterminated[F[_]: Functor, A](
queue: Queue[F, A],
limit: Int
): Stream[F, A] = fromQueueUnterminated(queue: QueueSource[F, A], limit)

(also, shouldn't the second one be private[fs2]?)

@armanbilge
Copy link
Copy Markdown
Member Author

armanbilge commented Jul 12, 2021 •
edited
Loading

@kubukoz I would have liked to, but marking it as private[fs2] still disappears the static forwarders :(

In fact, this was the problem in the original PR #2465.

@armanbilge armanbilge mentioned this pull request Aug 16, 2021
Merged
@armanbilge armanbilge mentioned this pull request Sep 18, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mpilquist mpilquist mpilquist approved these changes

@diesalbla diesalbla diesalbla approved these changes

@vasilmkd vasilmkd vasilmkd approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@armanbilge @arosien @kubukoz @mpilquist @diesalbla @vasilmkd