Skip to content
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

chore: use tokio instead of async-std #5828

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kamuik16
Copy link
Contributor

Description

ref #4449

Copy link
Member

@dariusc93 dariusc93 left a comment

Choose a reason for hiding this comment

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

LGTM. Left a question on whether we should use tokio::test macro instead for the tests.

Comment on lines +141 to +143
let rt = Runtime::new().unwrap();
rt.spawn(server);
rt.block_on(client);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we could use the macros feature to add tokio::test to the function and spawn the task for the server future while awaiting on the client without needing to call block_on.

Dont believe there would be any impact but we could probably poll both futures concurrently in the test to drive them to completion, or at least until the client future is completed, no? Whats your thoughts on that?

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Krishang, I'll let you address Darius suggestion

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