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 'static lifetime bound on http1/2 client IO #3667

Merged
merged 4 commits into from
Jul 1, 2024

Conversation

jgraef
Copy link
Contributor

@jgraef jgraef commented May 13, 2024

The method hyper::client::conn::http1::Builder::handshake has a 'static lifetime bound on the type parameter T (the underlying connection). This lifetime bound is not necessary and prohibits users of the library to borrowing connections to a hyper client.

There are similar bounds on the Connection struct, handshake method, plus all those for http2.

This PR removes these 'static lifetime bounds.

@jgraef
Copy link
Contributor Author

jgraef commented May 20, 2024

There's at least one 'static that I missed1. I will update this PR soon.

There are also some Unpin trait bounds that are not necessary. Should I open a new PR for them?

@dswij
Copy link
Member

dswij commented May 24, 2024

Probably related: #3595

I don't see any problem with this, unless I'm missing something? @seanmonstar

@jgraef
Copy link
Contributor Author

jgraef commented May 25, 2024

Should I update this PR with that one more 'static bound removed? I can also bring it up-to-date with master. Do you prefer rebase or merge?

@seanmonstar
Copy link
Member

Rebase or new commits, whichever you find easier to push. We can always squash when merging.

@seanmonstar
Copy link
Member

Seems like rustfmt isn't happy. Could you run rustfmt --check --edition 2021 $(git ls-files '*.rs')?

@seanmonstar seanmonstar merged commit 9580b35 into hyperium:master Jul 1, 2024
21 checks passed
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.

3 participants