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 IpAddr common methods #34694

Merged
merged 4 commits into from Jul 20, 2016
Merged

Add IpAddr common methods #34694

merged 4 commits into from Jul 20, 2016

Conversation

mathphreak
Copy link
Contributor

@mathphreak mathphreak commented Jul 6, 2016

Per rust-lang/rfcs#1668 (comment) no RFC is needed here.

The generated documentation for these methods is being weird. It shows a deprecation message referencing #27709 for each of them even though two of the referenced methods were stabilized as part of that issue. I don't know how best to address that.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@@ -59,6 +59,48 @@ pub enum Ipv6MulticastScope {
Global
}

impl IpAddr {
/// Returns true for the special 'unspecified' address (0.0.0.0 in IPv4, :: in IPv6).
pub fn is_unspecified(&self) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

These will all want to be marked unstable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sfackler Should I use feature="ip" and issue="27709" or should I use a different feature or issue? (#27709 and feature="ip" are what the Ipv4/v6Addr convenience methods already use.)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think reusing that issue and feature makes sense, unless @alexcrichton disagrees.

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 7, 2016
@alexcrichton
Copy link
Member

Thanks @mathphreak! Perhaps the documentation could also link to the relevant methods on Ipv4Addr and Ipv6Addr as well? Other than that looks good to me, I've tagged with T-libs to ensure this comes up during triage, but I suspect we'll all be ok with it.

cc @rust-lang/libs

@brson
Copy link
Contributor

brson commented Jul 7, 2016

lgtm

/// [IPv4]: ../../std/net/struct.Ipv4Addr.html#method.is_unspecified
/// [IPv6]: ../../std/net/struct.Ipv6Addr.html#method.is_unspecified
#[unstable(feature="ip", issue="27709",
reason="recently added and depends on unstable Ipv4Addr.is_unspecified()")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Per #27709 (comment), ipv4Addr.is_unspecified shouldn't be unstable (PR #34739 for stabilization).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@therealbstern I'll keep an eye out for that PR and update this once it gets merged.

bors added a commit that referenced this pull request Jul 13, 2016
Mark Ipv4Addr is_unspecified as stable and provide reference.

Per [#27709 (comment)](#27709 (comment)), no RFC is needed here.

IPv4 "unspecified" has been defined in [Stevens], and has been part of the IPv4 stack for quite some time.  This property should become stable, since this use of 0.0.0.0 is not going anywhere.

[Stevens][_UNIX Network Programming Volume 1, Second Edition_.  Stevens, W. Richard.  Prentice-Hall, 1998.  p. 891]

Please let me know if I got the rustdoc wrong or something.  I tried to be as terse as possible while still conveying the appropriate information.

This also has a slight impact on PR #34694, but that one came first, so this shouldn't block it, IMO.
@aturon
Copy link
Member

aturon commented Jul 13, 2016

I'm fine with these changes.

@alexcrichton
Copy link
Member

Discussed today during libs triage we decided to merge, thanks again for the PR @mathphreak!

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 19, 2016

📌 Commit 58da5dd has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jul 19, 2016

⌛ Testing commit 58da5dd with merge e15afbc...

bors added a commit that referenced this pull request Jul 19, 2016
Add IpAddr common methods

Per rust-lang/rfcs#1668 (comment) no RFC is needed here.

The generated documentation for these methods is being weird. It shows a deprecation message referencing #27709 for each of them even though two of the referenced methods were stabilized as part of that issue. I don't know how best to address that.
@bors
Copy link
Contributor

bors commented Jul 19, 2016

💔 Test failed - auto-win-msvc-32-opt

@alexcrichton
Copy link
Member

@bors: retry

On Tue, Jul 19, 2016 at 12:08 PM, bors notifications@github.com wrote:

💔 Test failed - auto-win-msvc-32-opt
https://buildbot.rust-lang.org/builders/auto-win-msvc-32-opt/builds/4123


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34694 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95KxdeUJkoNVpmf13MMooNzKAQFw8ks5qXSDAgaJpZM4JGlo0
.

@bors
Copy link
Contributor

bors commented Jul 20, 2016

⌛ Testing commit 58da5dd with merge d82b3cc...

@bors
Copy link
Contributor

bors commented Jul 20, 2016

💔 Test failed - auto-win-msvc-64-opt-rustbuild

@alexcrichton
Copy link
Member

@bors: retry

On Tue, Jul 19, 2016 at 9:15 PM, bors notifications@github.com wrote:

💔 Test failed - auto-win-msvc-64-opt-rustbuild
https://buildbot.rust-lang.org/builders/auto-win-msvc-64-opt-rustbuild/builds/1821


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34694 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95G7Llj4WYWx87RUNYMgF-lZwfkzKks5qXaDPgaJpZM4JGlo0
.

@bors
Copy link
Contributor

bors commented Jul 20, 2016

⌛ Testing commit 58da5dd with merge 9d5965a...

bors added a commit that referenced this pull request Jul 20, 2016
Add IpAddr common methods

Per rust-lang/rfcs#1668 (comment) no RFC is needed here.

The generated documentation for these methods is being weird. It shows a deprecation message referencing #27709 for each of them even though two of the referenced methods were stabilized as part of that issue. I don't know how best to address that.
@bors bors merged commit 58da5dd into rust-lang:master Jul 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants