Skip to content

Conversation

@camshaft
Copy link
Contributor

@camshaft camshaft commented Jun 5, 2023

Description of changes:

We finally have enough PRs merged to use the generic event loop for the turmoil IO provider.

Call-outs:

Since the current turmoil API doesn't allow splitting the sockets, we still need a smaller event loop to both send and receive on the socket.

Testing:

The existing turmoil tests should be sufficient.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@camshaft camshaft force-pushed the camshaft/turmoil-upgrade branch 3 times, most recently from 41d79d4 to 8439b3d Compare June 14, 2023 03:16
@camshaft camshaft marked this pull request as ready for review June 14, 2023 03:19
@camshaft camshaft requested a review from jmayclin June 14, 2023 20:30

tokio::spawn(run_io(socket, rx_producer, tx_consumer));

let el = EventLoop {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: el feels a bit dense even though it has a super-local scope. Primarily because it makes me think of the el rune from Diablo II rather than "event loop", but perhaps others who didn't play Diablo II so much wouldn't have that problem 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you say that like it's a bad thing 😄

@camshaft camshaft force-pushed the camshaft/turmoil-upgrade branch from 8439b3d to d41686f Compare June 15, 2023 03:26
@camshaft camshaft requested a review from jmayclin June 15, 2023 03:27
@camshaft camshaft enabled auto-merge (squash) June 15, 2023 03:41
@camshaft camshaft merged commit 3506661 into main Jun 15, 2023
@camshaft camshaft deleted the camshaft/turmoil-upgrade branch June 15, 2023 03:53
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