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

add TlsAcceptor::with_acceptor method #221

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

MarinPostma
Copy link
Contributor

This PR add a AcceptorBuilder::with_connector method that takes a generic acceptor implementing Accept.
This is necessary when you need to work with any other acceptor than AddrIncomming.

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mostly makes sense to me. I'd prefer to keep the bounds in the impl definition where possible (impl<C: AsyncRead + AsyncWrite + Unpin> TlsStream<c>).

Do you have a sense of whether this is semver-compatible? If not, can you add a semver-breaking version bump in a separate commit?

Out of curiosity, what do you (want to) use instead of AddrIncoming?

@MarinPostma
Copy link
Contributor Author

@djc thanks, I will do as suggested

We're abstracting networking in our project to implement simulation with turmoil. So I have a custom acceptor that wraps a turmoil TcpStream

@MarinPostma
Copy link
Contributor Author

as for semver, I'm not completely certain. It seems to me that yes, thanks to the default types, but I'm not an expert enough on semver.

@djc
Copy link
Member

djc commented Sep 15, 2023

I think this documentation says it should be okay.

@MarinPostma
Copy link
Contributor Author

@djc, I fixed fmt and fixed the trait bounds 👍

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@djc djc requested a review from cpu September 18, 2023 07:46
@djc
Copy link
Member

djc commented Sep 18, 2023

@cpu do you want to take a look?

Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@djc djc merged commit 163b3f5 into rustls:main Sep 18, 2023
9 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