-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Merge changes from next to master branch #2315
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
Conversation
[package.metadata.cargo_check_external_types] | ||
allowed_external_types = [ | ||
"tonic::*", | ||
"futures_core::stream::Stream", |
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.
We should pull from tokio_stream
instead
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.
@arjan-bal @dfawley @easwars if one of you can get to this simple fix that would be good, I am going to merge this anyways now.
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.
tokio_stream
re-exports futures_core::Stream
, so are these just equivalent?
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.
Yes, they are except that the tokio org controls the tokio_stream crate and I would rather depend on that than on the futures set of crates. (I have publish access for tokio_stream etc)
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.
LGTM
No description provided.