Add partial_min/max to libcore/cmp #17482
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon. |
| match v1.partial_cmp(&v2) { | ||
| Some(Less) | Some(Equal) => Some(v1), | ||
| Some(Greater) => Some(v2), | ||
| _ => ::option::None |
There was a problem hiding this comment.
Could you explicitly match None here and return just None as well? (the ::option:: shouldn't be necessary)
|
Previous discussion can be seen here: #16067, but the summary is that this was the conclusion about the best way to go about this if we were to go about this. I'm personally ok with adding these as freestanding functions. Given what we have today, these make sense, and if they're introduced as |
| } | ||
|
|
||
| for &(a, b, result) in data_float.iter() { | ||
| assert!(partial_min(a, b) == result); |
|
@thestinger I'm sorry, I don't see the problem. Is it about indentation or trailing whitespace? Edit: Fixed that. |
|
@alexcrichton Apparently, bors wasn't convinced, but, reading two logs, I can't determine what exactly caused it to fail and if it is my 'fault'. It doesn't have to do with my rebase, right? |
|
The bit near the bottom has the culprit: |
|
Oh, I read that as a warning. Are there any other non-obvious things it refuses to merge? |
Add partial_min/max to libcore/cmp Match against None and mark as experimental Shortened documentation. Removed whitespace
Intended to prevent each user to write his own partial_min/max, possibly differing in slight details. @sfackler encouraged to PR this on IRC. (Let's hope this works... First PR.)
Intended to prevent each user to write his own partial_min/max, possibly differing in slight details. @sfackler encouraged to PR this on IRC.
(Let's hope this works... First PR.)