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 two functions to check type of given address #36707

Merged
merged 1 commit into from
Oct 11, 2016

Conversation

achanda
Copy link
Contributor

@achanda achanda commented Sep 25, 2016

The is_v4 function returns true if the given IP is v4. The is_v6
function returns true if the IP is v6.

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@achanda achanda force-pushed the ip_type branch 2 times, most recently from faf7dbb to 61fe7a6 Compare September 25, 2016 01:56
@achanda
Copy link
Contributor Author

achanda commented Sep 25, 2016

@alexcrichton I was looking for a way to filter a list of IpAddrs based on type. I thought these two functions might help. But I have a feeling now that these might not be idiomatic enough. Thoughts?

@alexcrichton
Copy link
Member

Thanks for the PR @achanda! This seems pretty reasonable to me. I'd be fine even adding SocketAddr::{is_ipv4, is_ipv6} as well.

Thoughts @rust-lang/libs ?

Also looks like the travis failure may need to be resolved before landing.

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Sep 26, 2016
@achanda
Copy link
Contributor Author

achanda commented Sep 26, 2016

@alexcrichton makes sense, I will have a PR up for those as well once I fix these tests here.

@achanda achanda force-pushed the ip_type branch 2 times, most recently from 2b28259 to def4ed7 Compare September 27, 2016 17:05
@achanda
Copy link
Contributor Author

achanda commented Sep 29, 2016

Looks like this needs a r+ :)

@alexcrichton
Copy link
Member

@achanda sure yeah, just postponing to discuss the new feature in a libs triage

@achanda
Copy link
Contributor Author

achanda commented Sep 29, 2016

Ah, sorry for rushing!

@alexcrichton
Copy link
Member

@rfcbot fcp merge

@alexcrichton
Copy link
Member

I think the last piece of functionality we'd want here is a tracking issue and a specific #[unstable] tag on these methods (apart from the "catch all bucket" unstable issue for these methods), but that can happen once libs has taken a peek.

@rfcbot
Copy link

rfcbot commented Oct 3, 2016

FCP proposed with disposition to merge. Review requested from:

No concerns currently listed.
See this document for info about what commands tagged team members can give me.

@achanda
Copy link
Contributor Author

achanda commented Oct 4, 2016

@alexcrichton sorry, should I add the #[unstable] tag now? Not sure of the workflow, the rfcbot is new to me! :)

@alexcrichton
Copy link
Member

@achanda sure if you'd like! This is likely to be accepted so if you want to go ahead and open an issue for this as well I can tag it and you can reference it.

Also don't worry, rfcbot is hot new technology for us too!

The is_v4 function returns true if the given IP is v4. The is_v6
function returns true if the IP is v6.
@achanda
Copy link
Contributor Author

achanda commented Oct 4, 2016

@alexcrichton done!

@alexcrichton
Copy link
Member

@bors: r+

Thanks again @achanda

@bors
Copy link
Contributor

bors commented Oct 10, 2016

📌 Commit 80a7a3c has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Oct 11, 2016

⌛ Testing commit 80a7a3c with merge ead9212...

bors added a commit that referenced this pull request Oct 11, 2016
Add two functions to check type of given address

The is_v4 function returns true if the given IP is v4. The is_v6
function returns true if the IP is v6.
@bors bors merged commit 80a7a3c into rust-lang:master Oct 11, 2016
@achanda achanda deleted the ip_type branch October 13, 2016 00:18
@rfcbot
Copy link

rfcbot commented Oct 17, 2016

@alexcrichton proposal cancelled.

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.

5 participants