-
Notifications
You must be signed in to change notification settings - Fork 1k
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: Remove futures crates #1478
Conversation
tonic/Cargo.toml
Outdated
@@ -50,7 +50,7 @@ channel = [] | |||
[dependencies] | |||
base64 = "0.21" | |||
bytes = "1.0" | |||
futures-util = {version = "0.3", default-features = false} | |||
pin-utils = "0.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use https://docs.rs/tokio/latest/tokio/macro.pin.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, tokio
has been used behind transport
feature. To use tokio::pin
macro, we have to use tokio
throughout tonic
implementations. I think tonic
is mostly used with tokio
so it seems not to be really matter. However, as I'm not sure whether it is appropriate to always depend on tokio
only for using tokio::pin
, I would like to hear your opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we depend on tokio's default features (which should be basically empty) then it should be available and pretty light weight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviews! I'll fix to use tokio
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
7169064
to
fb1cd13
Compare
Thanks! |
Motivation
Completes of removing futures crates.
Solution
futures_{core, util}::ready
withstd::task::ready
, which was stabilized at Rust 1.64.pin_utils
crate directly instead of viafutures_util
. (Whentonic
's MSRV be bumped to Rust1.68
in the future,std::pin::pin
can be used.)