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

ready_cache: just use pin_project for Pending #667

Merged
merged 3 commits into from
Jun 17, 2022

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Jun 16, 2022

This gets rid of the Unpin impl with the weird comment on it.

Alternatively, we could just put a S: Unpin bound on Pending, but
this changes the public API to require that the service type is Unpin.
In practice, it will be, but we could also just avoid the trait bound.

Signed-off-by: Eliza Weisman eliza@buoyant.io

This gets rid of the `Unpin` impl with the weird comment on it.

Alternatively, we could just put a `S: Unpin` bound on `Pending`, but
this changes the public API to require that the service type is `Unpin`.
In practice, it will be, but we could also just avoid the trait bound.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw requested review from olix0r and LucioFranco June 16, 2022 17:04
@LucioFranco
Copy link
Member

I don't understand the motivation for this change? Is it to just remove the unsafe impl?

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw merged commit 45a13b1 into master Jun 17, 2022
@hawkw hawkw deleted the eliza/pin-project-ready-cache branch June 17, 2022 16:15
hawkw added a commit that referenced this pull request Jun 17, 2022
This gets rid of the `Unpin` impl with the weird comment on it.

Alternatively, we could just put a `S: Unpin` bound on `Pending`, but
this changes the public API to require that the service type is `Unpin`.
In practice, it will be, but we could also just avoid the trait bound.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Jun 17, 2022
This gets rid of the `Unpin` impl with the weird comment on it.

Alternatively, we could just put a `S: Unpin` bound on `Pending`, but
this changes the public API to require that the service type is `Unpin`.
In practice, it will be, but we could also just avoid the trait bound.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
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