Skip to content

Conversation

@Kimundi
Copy link
Contributor

@Kimundi Kimundi commented Nov 8, 2013

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 Results 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

@alexcrichton
Copy link
Member

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!

@alexcrichton
Copy link
Member

I've added this to the minutes of the next meeting.

@alexcrichton
Copy link
Member

erm, agenda

Copy link
Contributor

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)

Copy link
Contributor Author

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().

Copy link
Contributor

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

@Kimundi
Copy link
Contributor Author

Kimundi commented Nov 23, 2013

Did this get discussed at the last meeting?

@alexcrichton
Copy link
Member

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.

@gsemet
Copy link

gsemet commented Nov 27, 2013

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.

@alexcrichton
Copy link
Member

It was decided in the meeting this week that we'd like to accept this, so feel free to ping me when rebased!

bors added a commit that referenced this pull request Dec 7, 2013
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
@bors bors closed this Dec 7, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants