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

Remove ServeFuture #2977

Closed
wants to merge 1 commit into from
Closed

Conversation

mladedav
Copy link
Collaborator

Motivation

The custom future type did nothing, it just passed through the poll method to the inner boxed future. The only thing I could tell is different is that it implemented Debug but I'm not convinced that's something that a future type needs anyway.

The commit that added this custom future called this a preparation for graceful shutdown which has been since implemented and does not need this so I don't think this deletion would cause trouble later.

Solution

I removed the code and let Serve::into_future return directly the Pin<Box<dyn Future<Output = _>>>.

@mladedav mladedav requested a review from jplatte October 11, 2024 13:54
@jplatte
Copy link
Member

jplatte commented Oct 11, 2024

This sort of thing is commonly done to allow changing the inner type in a non-breaking release. Is there any advantage to using Pin<Box<_>> directly, other than reducing the amount of code a bit?

@mladedav
Copy link
Collaborator Author

mladedav commented Oct 11, 2024

I don't think there should be any difference, I just noticed the code doesn't actually do anything so I tried removing it. We can keep it for the flexibility then.

@mladedav mladedav closed this Oct 11, 2024
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