Skip to content
Please note that GitHub no longer supports Internet Explorer.

We recommend upgrading to the latest Microsoft Edge, Google Chrome, or Firefox.

Learn more
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

Implement an async version of ToSocketAddrs #74

Merged
merged 14 commits into from Sep 4, 2019
Merged

Conversation

@DCjanus
Copy link
Contributor

DCjanus commented Aug 18, 2019

Resolve #50

src/net/addr.rs Outdated Show resolved Hide resolved
Copy link
Member

stjepang left a comment

Thank you for the PR! I think we only need to replace the Output associated type with a "hack" that simulates async fn in traits.

src/net/addr.rs Outdated Show resolved Hide resolved
src/net/addr.rs Outdated Show resolved Hide resolved
@DCjanus

This comment has been minimized.

Copy link
Contributor Author

DCjanus commented Aug 20, 2019

Force pushed to rebase on master

@stjepang Hi, after a long day, I've created a commit to replace std::net::ToSocketAddrs with async_std::net::ToSocketAddrs, would you mind review again. Thanks.

@stjepang

This comment has been minimized.

Copy link
Member

stjepang commented Aug 26, 2019

Could you perhaps incorporate changes from https://github.com/DCjanus/async-std/pull/1 here? In particular, let's remove the new re-exports and move tests into the tests directory. At the same time, let's also delete unit tests that are not relevant to ToSocketAddrs.

@skade
skade approved these changes Aug 26, 2019
src/net/addr.rs Outdated Show resolved Hide resolved
src/net/addr.rs Outdated Show resolved Hide resolved
tests/addr.rs Outdated Show resolved Hide resolved
tests/addr.rs Outdated Show resolved Hide resolved
tests/addr.rs Outdated Show resolved Hide resolved
tests/addr.rs Outdated Show resolved Hide resolved
tests/addr.rs Outdated Show resolved Hide resolved
src/net/addr.rs Outdated Show resolved Hide resolved
src/net/addr.rs Show resolved Hide resolved
tests/addr.rs Outdated Show resolved Hide resolved
src/net/addr.rs Outdated Show resolved Hide resolved
src/net/addr.rs Outdated Show resolved Hide resolved
src/net/addr.rs Outdated Show resolved Hide resolved
src/net/addr.rs Outdated Show resolved Hide resolved
src/net/addr.rs Outdated Show resolved Hide resolved
@skade

This comment has been minimized.

Copy link
Collaborator

skade commented Sep 3, 2019

@stjepang is this ready to merge?

Copy link
Member

stjepang left a comment

Some small tweaks are needed and then we'll be ready to merge.

src/net/addr.rs Outdated Show resolved Hide resolved
src/net/addr.rs Outdated Show resolved Hide resolved
docs/src/tutorial/accept_loop.md Show resolved Hide resolved
Copy link
Member

stjepang left a comment

This looks good to me now. Thanks for driving the PR to completion

@stjepang

This comment has been minimized.

Copy link
Member

stjepang commented Sep 4, 2019

bors r+

bors bot added a commit that referenced this pull request Sep 4, 2019
Merge #74
74: Implement an async version of ToSocketAddrs r=stjepang a=DCjanus

Resolve #50 

Co-authored-by: DCjanus <dcjanus@dcjanus.com>
Co-authored-by: Stjepan Glavina <stjepang@gmail.com>
@stjepang stjepang merged commit 238a3c8 into async-rs:master Sep 4, 2019
6 of 8 checks passed
6 of 8 checks passed
Travis CI - Branch Build Errored
Details
bors Canceled
Details
Header rules No header rules processed
Details
Pages changed All files already uploaded
Details
Redirect rules No redirect rules processed
Details
Mixed content No mixed content detected
Details
Travis CI - Pull Request Build Passed
Details
deploy/netlify Deploy preview ready!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.