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

fix: Make full use of the tokio ecosystem if the tokio feature is enabled on Unix #336

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

DataTriny
Copy link
Member

I noticed that we can get rid of a couple of async-related crates when the tokio feature is enabled, because tokio ships its own async types.

@mwcampbell
Copy link
Contributor

I noticed that the following line was removed in the select loop:

complete => return Ok(()),

I'm not sure I understand what that line did. Was it not needed in the first place, or is there some other change that makes it unnecessary even for async-io?

@DataTriny
Copy link
Member Author

DataTriny commented Jan 8, 2024

In practice this line is never executed since the app will terminate before both streams close. Since tokio's version of select doesn't allow this "complete" keyword, I decided to remove it. I think we could put it back if you want, with #[cfg(not(feature = "tokio))].

@mwcampbell
Copy link
Contributor

If it's never needed in practice, I'm fine with leaving it out.

@mwcampbell mwcampbell merged commit c034802 into main Jan 8, 2024
5 checks passed
@mwcampbell mwcampbell deleted the unix-tokio-deps branch January 8, 2024 20:44
@mwcampbell mwcampbell mentioned this pull request Jan 8, 2024
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