Skip to content

make all combinator futures opaque #509

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

Merged
merged 4 commits into from
Jan 6, 2021
Merged

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Jan 6, 2021

For stability reasons, we probably don't want to expose future
combinator types from the futures_util crate in public APIs. If we
were to change which combinators are used to implement these futures, or
if futures_util made breaking changes, this could cause API breakage.

This branch wraps all publicly exposed futures_util types in the
opaque_future! macro added in #508, to wrap them in newtypes that hide
their internal details. This way, we can change these futures' internals
whenever we like.

For stability reasons, we probably don't want to expose future
combinator types from the `futures_util` crate in public APIs. If we
were to change which combinators are used to implement these futures, or
if `futures_util` made breaking changes, this could cause API breakage.

This branch wraps all publicly exposed `futures_util` types in the
`opaque_future!` macro added in #508, to wrap them in newtypes that hide
their internal details. This way, we can change these futures' internals
whenever we like.
@hawkw hawkw requested a review from LucioFranco January 6, 2021 20:32
@hawkw hawkw mentioned this pull request Jan 6, 2021
2 tasks
hawkw added 3 commits January 6, 2021 12:37
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
@hawkw hawkw requested a review from olix0r January 6, 2021 21:58
@hawkw hawkw merged commit 3b7c91e into master Jan 6, 2021
@hawkw hawkw deleted the eliza/make-futures-opaque branch January 6, 2021 23:06
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