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

rename ToSocketAddr to ToSocketAddrs #60

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

MarinPostma
Copy link
Contributor

Rename ToSocketAddr to ToSocketAddrs to make turmoil more compatible with tokio.

My motivation is that I am trying to mock parts of tokio with turmoil in my project, behind a feature flags. This is only possible is the crates are at least name-compatible.

@mcches
Copy link
Contributor

mcches commented Nov 22, 2022

The turmoil implementation is explicitly singleton v. plural like the tokio one. I didn't have issues integrating due to the supplied From conversions. One nice feature we have here is that human readable hostnames are supported, which makes debugging via the trace a lot easier.

I'm not opposed since the trait is opaque anyways and we can do whatever we like internally.

Could you share an example snippet that was troublesome?

@MarinPostma
Copy link
Contributor Author

MarinPostma commented Nov 24, 2022

Consider a function like:

async fn setup_server(addrs: impl ToSocketAddrs) {
     let listener = TcpListener::bind(addrs).await;
//...
}

@mcches
Copy link
Contributor

mcches commented Nov 29, 2022

Consider a function like:

async fn setup_server(addrs: impl ToSocketAddrs) {
     let listener = TcpListener::bind(addrs).await;
//...
}

I see. Given the nature of that trait I generally see a concrete addr passed in this use case and then the From conversions do their job.

I'm good with aligning the trait, but we have to bump the major version so it might be worth stashing this and getting some of the net gaps fleshed out with minor version releases if you can unblock yourself.

We also should export as turmoil::net::ToSocketAddrs to align.

Copy link
Contributor

@mcches mcches left a comment

Choose a reason for hiding this comment

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

lgtm!

@mcches mcches merged commit 2c0423d into tokio-rs:main Jan 5, 2023
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