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

Migrate to Tokio 0.2 #603

Merged
merged 73 commits into from
Jan 11, 2021
Merged

Migrate to Tokio 0.2 #603

merged 73 commits into from
Jan 11, 2021

Conversation

Xanewok
Copy link
Contributor

@Xanewok Xanewok commented Dec 16, 2020

Motivated to migrate RLS off Tokio 0.1 for IPC transport but since we're at it, it might make sense to migrate everything else in one fell swoop.

This is a very early draft but opening it now to prevent duplicating work in case someone else starts working in it (or would like to help 😁).

Fixes #582
Fixes #485

Rather than separately pulling 0.2 and 0.4 versions.
Otherwise we can't return a future due to borrow on Endpoint::incoming
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

lgtm!

core-client/transports/src/transports/ws.rs Show resolved Hide resolved
@Xanewok Xanewok requested review from NikVolf and niklasad1 January 5, 2021 11:20
@Xanewok
Copy link
Contributor Author

Xanewok commented Jan 8, 2021

It seems this only needs a second, separate approval and should be good to go 🎉

@niklasad1
Copy link
Member

Yeah, I will review it, latest on Monday.


[dev-dependencies]
tokio = { version = "0.2", features = ["rt-core", "macros"] }
Copy link
Member

@niklasad1 niklasad1 Jan 11, 2021

Choose a reason for hiding this comment

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

these dev-dependencies will leaked into the release build, however rust-lang/cargo#8997 have been stabalized so should be included in the next Rust release I guess.

@@ -1,14 +1,14 @@
use std;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use std;

Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

Nice work LGTM, I didn't review it super carefully except the Windows mio hacks.

@Xanewok
Copy link
Contributor Author

Xanewok commented Jan 11, 2021

While going through the diff again I noticed that I removed a Tokio re-export in ipc server - rolled that back in a05cd4e by adding the re-export.

Since this is all green, I'm merging it. Thank you for taking your time to review this!

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.

JSON Neo Tokio 0.2 Switch to std::future::Future
4 participants