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

Modify UdpSocket::connect() to take SocketAddr by ref (#667) #731

Merged
merged 2 commits into from Nov 19, 2017

Conversation

Projects
None yet
3 participants
@bobbo
Copy link

bobbo commented Sep 19, 2017

Change argument for UdpSocket::connect() to take &SocketAddr. This is more consistent with other UdpSocket methods, plus the TcpSocket::connect() method. Closes issue #667.

@KodrAus

This comment has been minimized.

Copy link

KodrAus commented Sep 19, 2017

Thanks @bobbo! This looks good to me 👍

@carllerche

This comment has been minimized.

Copy link
Owner

carllerche commented Sep 19, 2017

This is unfortunately a breaking change :(

So, this can't be merged until an 0.7 release is planned. or UdpSocket::connect is deprecated in favor of UdpSocket::connect2 (and it is switched back or 0.7).

@carllerche carllerche added this to the v0.7 milestone Oct 11, 2017

@carllerche

This comment has been minimized.

Copy link
Owner

carllerche commented Nov 18, 2017

If you are up for it, could you rebase this against the v0.7.x branch? I am using that as staging for v0.7.x

@bobbo bobbo force-pushed the bobbo:issue-667 branch from f677e29 to 55633d4 Nov 18, 2017

@bobbo bobbo changed the base branch from master to v0.7.x Nov 18, 2017

@bobbo bobbo force-pushed the bobbo:issue-667 branch from 55633d4 to 059bcab Nov 18, 2017

@bobbo

This comment has been minimized.

Copy link
Author

bobbo commented Nov 19, 2017

Hi @carllerche, I've rebased on top of v0.7.x and changed PR target branch.

@carllerche carllerche merged commit 667b7a2 into carllerche:v0.7.x Nov 19, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.