-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Made Results API more composable #10364
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
Conversation
Hm, I'd like to discuss this in a meeting because I think we want to settle on the result API now instead of later. Next week we're not having a meeting, so this would have to sit for awhile before we get to officially discuss it. I'm personally in favor of this change, though, I like the direction it went in! |
I've added this to the minutes of the next meeting. |
erm, agenda |
/// Unwraps a result, yielding the content of an `Err`. | ||
/// Fails if the value is an `Ok`. | ||
#[inline] | ||
pub fn err_get(self) -> E { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_err
seems like a better name to me, and more consistent with every other function (get_ptr, get_mut, etc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea behind that naming is that err_get
is shorthand for err().get()
, just like for Option
take_unwrap
is shorthand for take().unwrap()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_err() is a better name, from a user view
Did this get discussed at the last meeting? |
This did not, but it'll remain on the agenda until it does. I'm still very much in favor of this, we just need some more opinions to weigh in on this. |
I +1 this PR, however, is it possible to write a document about the naming convention to apply in all module? It's more than a Coding Style guide, it's more a general reference page where one would find the convention to apply. Because I cannot look at the different "convention" used in different rust modules to write my own. I've setup this kind of naming / API convention document in my work, and this was quite successful. One would follow this doc for new development, and if a change in the convention occurs (changes will occur), it's more easier to find out "what is good/what is bad" at first sight. |
It was decided in the meeting this week that we'd like to accept this, so feel free to ping me when rebased! |
This implements parts of the changes to `Result` and `Option` I proposed and discussed in this thread: https://mail.mozilla.org/pipermail/rust-dev/2013-November/006254.html This PR includes: - Adding `ok()` and `err()` option adapters for both `Result` variants. - Removing `get_ref`, `expect` and iterator constructors for `Result`, as they are reachable with the variant adapters. - Removing `Result`s `ToStr` bound on the error type because of composability issues. (See https://mail.mozilla.org/pipermail/rust-dev/2013-November/006283.html) - Some warning cleanups
This implements parts of the changes to
Result
andOption
I proposed and discussed in this thread: https://mail.mozilla.org/pipermail/rust-dev/2013-November/006254.htmlThis PR includes:
ok()
anderr()
option adapters for bothResult
variants.get_ref
,expect
and iterator constructors forResult
, as they are reachable with the variant adapters.Result
sToStr
bound on the error type because of composability issues. (See https://mail.mozilla.org/pipermail/rust-dev/2013-November/006283.html)