Skip to content

Conversation

Heap-Hop
Copy link

#25

@pchickey
Copy link
Contributor

Thanks for submitting this.

We will want to exercise this with a test like we do for the tcp_echo_server.

As far as the style, this uses futures for StreamTasks and we were considering recommending futures-concurrency (#17) for this sort of thing - would you be up for translating it to use that?

@Heap-Hop
Copy link
Author

we were considering recommending futures-concurrency

Done.

It does need a test, but due to bytecodealliance/wasmtime#9667, I haven’t found a good way to write one yet.


futures_lite::future::poll_fn(|cx| {
if let Poll::Ready(_) = listening_task.as_mut().poll(cx) {
return Poll::Ready(());
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want to return as Ready until both the Listener is ready and the stream tasks are all complete.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I didn't quite understand what you meant. Could you please tell me your thoughts directly?

@pchickey
Copy link
Contributor

Sorry, working on this crate fell off my radar for a bit as I started a new job.

I have merged the my rewrite of the reactor and streams abstractions (#35, #37). I'm now back to looking at the work you did while I was focusing on that work.

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.

2 participants