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

Propose RFC for IpAddr common methods #1668

Closed
wants to merge 3 commits into from

Conversation

mathphreak
Copy link

@eefriedman
Copy link
Contributor

Please include the exact signature of the proposed methods in the proposal... something like this:

impl IpAddr {
    fn is_loopback(&self) -> bool { ... }
    fn is_multicast(&self) -> bool { ... }
}

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Jul 6, 2016
@mathphreak
Copy link
Author

@eefriedman Just added that.

@Ryman
Copy link

Ryman commented Jul 6, 2016

Similar suggestion (without required details) #1554 cc @canndrew

@sfackler
Copy link
Member

sfackler commented Jul 6, 2016

It looks like there are some other shared methods between the two types (is_documentation, etc). Seems like we should probably add them all at once?

@mathphreak
Copy link
Author

@sfackler I only mentioned the stable methods. If it would be better to list all of them, I can make that change.

@sfackler
Copy link
Member

sfackler commented Jul 6, 2016

Yeah I figure we should add all of them - these new methods would land unstable anyway.

@mathphreak
Copy link
Author

@sfackler Done.

Also, I noticed that Ipv4Addr has an is_link_local() method, while Ipv6Addr has an is_unicast_link_local() method. It seems like those are nearly the same; should I add an is_link_local() as well?

@sfackler
Copy link
Member

sfackler commented Jul 6, 2016

I don't think I know enough about IPv6 to be able to answer that :)

@mathphreak
Copy link
Author

Neither do I. Unless an IPv6 expert shows up and says that those are the same, I think it's best to leave is_link_local() out.

@alexcrichton
Copy link
Member

I'm actually surprised to see this RFC because I thought these methods already exists! I could have sworn we either had PR or an issue about this...

@mathphreak this is actually the kind of addition where the libs team generally doesn't require an RFC. If you'd like to close this out and send a PR directly to rust-lang/rust proposing this change, I suspect we'd merge pretty quickly!

@mathphreak
Copy link
Author

@alexcrichton I wasn't quite sure if that was the case. I'll get right on that. I'll leave this open until I submit that PR.

@mathphreak
Copy link
Author

That took longer than expected, but rust-lang/rust#34694 is now live.

@mathphreak mathphreak closed this Jul 6, 2016
@canndrew
Copy link
Contributor

canndrew commented Jul 7, 2016

Thanks @Ryman, I've closed that bug report.
Edit: Also thanks @mathphreak for doing what I was too lazy to do.

bors added a commit to rust-lang/rust 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 added a commit to rust-lang/rust 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.
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 RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants