PartialEq and PartialOrd between IpAddr and Ipv[46]Addr. #38464
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (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. |
clarfonthey
force-pushed
the
ip_cmp
branch
2 times, most recently
from
abeb161
to
3a3aa84
Compare
frewsxcv
added
the
T-libs-api
clarfonthey
changed the title
|
Looks good to me, thanks for the PR! @rfcbot fcp merge |
|
Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
|
Can this lead to confusion down the line? IPv4 addresses can be mapped into IPv6 ones, and so a user might expect e.g. |
|
@lfairy that doesn't match the current implementation of I tried coding a version of that type of match and I got: https://is.gd/6Yzorg |
|
@clarcharr My point isn't that fuzzy matching is a good idea (it probably isn't), but that a user may presume that it works that way. To someone who works on dual-stack IPv4/IPv6 code, the idea that |
|
@lfairy I feel like that's mostly a documentation change, though. it makes sense to me to document it if there is any fuzzy, inexact equality, but if it's just pure equality, I don't feel like that requires any documentation. right now that expression will be a compiler error |
|
@brson @BurntSushi speak now or forever hold your peace @bors r+ |
|
📌 Commit 9301e2e has been approved by |
|
⌛ Testing commit 9301e2e with merge 72616b1... |
|
💔 Test failed - status-travis |
|
… On Tue, Jan 17, 2017 at 6:19 PM, bors ***@***.***> wrote:
💔 Test failed - status-travis
<https://travis-ci.org/rust-lang/rust/builds/192881724>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#38464 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95Hd7suA-1YC5EBJJ7hBlgTIyltJ9ks5rTXatgaJpZM4LQWzm>
.
|
|
⌛ Testing commit 9301e2e with merge 90a9c8c... |
|
💔 Test failed - status-appveyor |
|
@bors: retry
* network timeout to github
…On Wed, Jan 18, 2017 at 12:59 AM, bors ***@***.***> wrote:
💔 Test failed - status-appveyor
<https://ci.appveyor.com/project/rust-lang/rust/build/1.0.1598>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#38464 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95IkdK3Q6szFiJFJO6wR8cmUwNqy5ks5rTdRtgaJpZM4LQWzm>
.
|
|
@alexcrichton, |
|
@KalitaAlexey I think that that issue is Appveyor's, not Rust's |
|
@KalitaAlexey perhaps! I know just about as much as you probably do about AppVeyor, and PRs are always welcome! |
|
⌛ Testing commit 9301e2e with merge 47daef8... |
|
💔 Test failed - status-travis |
|
… On Wed, Jan 18, 2017 at 3:17 PM, bors ***@***.***> wrote:
💔 Test failed - status-travis
<https://travis-ci.org/rust-lang/rust/builds/193205275>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#38464 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95N-S2uMS18pObyPy4sOzTXIOaPaSks5rTp1_gaJpZM4LQWzm>
.
|
|
⌛ Testing commit 9301e2e with merge 4cf483c... |
|
💔 Test failed - status-travis |
|
… On Wed, Jan 18, 2017 at 5:09 PM, bors ***@***.***> wrote:
💔 Test failed - status-travis
<https://travis-ci.org/rust-lang/rust/builds/193229675>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#38464 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95B_93cZtEKl0Y1nS2bqIIVQm3vO3ks5rTrezgaJpZM4LQWzm>
.
|
|
⌛ Testing commit 9301e2e with merge 74c42ac... |
PartialEq and PartialOrd between IpAddr and Ipv[46]Addr. PartialEq was rather useful, so, I figured that I'd implement it. I added PartialOrd for good measure.
|
☀️ Test successful - status-appveyor, status-travis |
PartialEq was rather useful, so, I figured that I'd implement it. I added PartialOrd for good measure.