Explain meaning of Result iters and link to factory functions #38158

sourcefrog · 2016-12-04T17:03:46Z

No description provided.

rust-highfive · 2016-12-04T17:03:53Z

r? @aturon

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

bluss · 2016-12-04T17:56:50Z

@bors r+ rollup

Thank you!

bors · 2016-12-04T17:56:51Z

📌 Commit b2849c7 has been approved by bluss

GuillaumeGomez

A few things to fix.

GuillaumeGomez · 2016-12-07T18:40:22Z

src/libcore/result.rs

@@ -501,6 +501,8 @@ impl<T, E> Result<T, E> {

    /// Returns an iterator over the possibly contained value.
    ///
+    /// The iterator yields one value if the result is `Ok`, otherwise none.


Missing Ok's url.

GuillaumeGomez · 2016-12-07T18:40:26Z

src/libcore/result.rs

@@ -520,6 +522,8 @@ impl<T, E> Result<T, E> {

    /// Returns a mutable iterator over the possibly contained value.
    ///
+    /// The iterator yields one value if the result is `Ok`, otherwise none.


Missing Ok's url.

GuillaumeGomez · 2016-12-07T18:40:30Z

src/libcore/result.rs

@@ -848,6 +852,8 @@ impl<T, E> IntoIterator for Result<T, E> {

    /// Returns a consuming iterator over the possibly contained value.
    ///
+    /// The iterator yields one value if the result is `Ok`, otherwise none.


Missing Ok's url.

GuillaumeGomez · 2016-12-07T18:40:48Z

src/libcore/result.rs

@@ -893,8 +899,13 @@ impl<'a, T, E> IntoIterator for &'a mut Result<T, E> {

 /// An iterator over a reference to the [`Ok`] variant of a [`Result`].
 ///
+/// The iterator yields one value if the result is `Ok`, otherwise none.


Missing Ok's url.

GuillaumeGomez · 2016-12-07T18:40:59Z

src/libcore/result.rs

-/// the [`IntoIterator`] trait).
+/// An iterator over the value in a [`Ok`] variant of a [`Result`].
+///
+/// The iterator yields one value if the result is `Ok`, otherwise none.


Missing Ok's url.

I'll add them - it wasn't clear to me if the intention is to link every reference...

GuillaumeGomez · 2016-12-07T18:41:20Z

@bors: r-

Rollup of 9 pull requests - Successful merges: #38085, #38123, #38151, #38153, #38158, #38163, #38186, #38189, #38208 - Failed merges:

sourcefrog · 2016-12-11T17:24:52Z

links added, please take another look

sourcefrog · 2016-12-11T20:20:48Z

I'm not sure why https://travis-ci.org/rust-lang/rust/jobs/183046022 failed, maybe just a glitch?

GuillaumeGomez · 2016-12-11T21:15:49Z

Don't worry, travis build is broken for the moment, so if no error/failed test, it's good. Can you squash your commits please? If you don't know how to do it, take a look here. Once done, ping me and I merge.

sourcefrog · 2016-12-13T19:22:07Z

Don't worry, travis build is broken for the moment, so if no error/failed test, it's good. Can you squash your commits please? If you don't know how to do it, take a look here. Once done, ping me and I merge.

Will do.

Should we teach Bors to squash-and-merge?

sourcefrog · 2016-12-13T19:30:12Z

Squashed, and rebased to current master.

GuillaumeGomez · 2016-12-14T09:57:19Z

src/libcore/result.rs

@@ -848,6 +856,8 @@ impl<T, E> IntoIterator for Result<T, E> {

    /// Returns a consuming iterator over the possibly contained value.
    ///
+    /// The iterator yields one value if the result is [`Ok`], otherwise none.


Otherwise None (with url).

GuillaumeGomez · 2016-12-14T09:57:39Z

src/libcore/result.rs

@@ -501,6 +501,8 @@ impl<T, E> Result<T, E> {

    /// Returns an iterator over the possibly contained value.
    ///
+    /// The iterator yields one value if the result is [`Ok`], otherwise none.


Otherwise None (with url).

Well, no, it doesn't yield None, it just yields no values at all.

I realize this is implemented by next() returning None but I think saying that would only make this confusing. I could rephrase it as "the iterator's next method returns Some of the inner and then None if the result is Ok, otherwise it returns None." But that seems to be needlessly puncturing the iterator abstraction.

A couple of general comments:

There are lots of references to well-known types such as None and Ok throughout the code without hyperlinks, indeed even in this very file. If the goal is that every type be manually linked maybe that should go into CONTRIBUTING.md to avoid make that clear. IMO it is not so necessary to linkify values the reader is highly likely to already understand, and that anyhow are linked from nearby.

In my experience repeatedly asking contributors for small incremental changes to a PR that's already an improvement tends to discourage future contributions. My normal practice now is to accept the change and encourage them to send another PR making additional cleanups, and I think this gives them a better feeling about contributing.

No, it returns None, not "no values at all".

Also, I'm currently finishing to add missing doc examples, then I'll go through all missing urls. None and every other struct/enum/etc... should be linked.

Also, even it seems minor for you, it isn't for me since all these minor things people don't add, I have to after. And it's getting quite huge.

I don't ask it just to bother you or for extreme and dark reasons, it's really because there is a need out of it.

Unless I'm misunderstanding it the iter will either have length 1 and contain the value inside the Ok result, or it will have length zero. It never yields None, unless the result is Ok(None). That's what I was trying to say by it yields 'no values at all'.

https://play.rust-lang.org/?gist=3d6a9500b782e878fae356a08888838b&version=stable&backtrace=0

Ok, I see your point. Confusion on my side. Sorry about that... Therefore, no change is needed.

No problem, I could have been confused myself so I appreciate you checking.

I can send you a little cleanup pr later to linkify other instances. It sure would be nice if we can make it automatic later.

It's been discussed for a while. You can take a look at this issue if you want to take a look at the current status.

GuillaumeGomez · 2016-12-14T09:58:17Z

src/libcore/result.rs

@@ -520,6 +524,8 @@ impl<T, E> Result<T, E> {

    /// Returns a mutable iterator over the possibly contained value.
    ///
+    /// The iterator yields one value if the result is [`Ok`], otherwise none.


Otherwise None (with url).

GuillaumeGomez · 2016-12-14T09:59:16Z

src/libcore/result.rs

@@ -893,8 +905,13 @@ impl<'a, T, E> IntoIterator for &'a mut Result<T, E> {

 /// An iterator over a reference to the [`Ok`] variant of a [`Result`].
 ///
+/// The iterator yields one value if the result is [`Ok`], otherwise none.


Otherwise None (with url).

GuillaumeGomez · 2016-12-14T09:59:24Z

src/libcore/result.rs

-/// the [`IntoIterator`] trait).
+/// An iterator over the value in a [`Ok`] variant of a [`Result`].
+///
+/// The iterator yields one value if the result is [`Ok`], otherwise none.


Otherwise None (with url).

GuillaumeGomez · 2016-12-15T10:12:51Z

@bors: r+ rollup

bors · 2016-12-15T10:12:52Z

📌 Commit 16d4b7b has been approved by GuillaumeGomez

Explain meaning of Result iters and link to factory functions

bors · 2016-12-16T11:46:42Z

⌛ Testing commit 16d4b7b with merge 02bb808...

bors · 2016-12-16T11:49:27Z

💔 Test failed - auto-win-msvc-64-opt

GuillaumeGomez · 2016-12-16T15:48:15Z

@bors: retry

bors · 2016-12-16T15:48:22Z

⌛ Testing commit 16d4b7b with merge f5fa85e...

bors · 2016-12-16T15:54:39Z

💔 Test failed - auto-mac-64-opt-rustbuild

alexcrichton · 2016-12-16T17:26:33Z

@bors: retry

…

On Fri, Dec 16, 2016 at 7:54 AM, bors ***@***.***> wrote: 💔 Test failed - auto-mac-64-opt-rustbuild <https://buildbot.rust-lang.org/builders/auto-mac-64-opt-rustbuild/builds/3275> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#38158 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD95GmHrclijQ3hIO3cl2XcXETlONVdks5rIrRBgaJpZM4LDokl> .

Explain meaning of Result iters and link to factory functions

sanxiyn · 2016-12-19T15:52:02Z

@bors retry

alexcrichton · 2016-12-20T18:41:45Z

@bors: retry

Explain meaning of Result iters and link to factory functions

Rollup of 29 pull requests - Successful merges: #37761, #38006, #38131, #38150, #38158, #38171, #38208, #38215, #38236, #38245, #38289, #38302, #38315, #38346, #38388, #38395, #38398, #38418, #38432, #38451, #38463, #38468, #38470, #38471, #38472, #38478, #38486, #38493, #38498 - Failed merges: #38271, #38483

rust-highfive assigned aturon Dec 4, 2016

GuillaumeGomez mentioned this pull request Dec 7, 2016

Rollup of 9 pull requests #38214

Merged

GuillaumeGomez requested changes Dec 7, 2016

View reviewed changes

bors added a commit that referenced this pull request Dec 7, 2016

Auto merge of #38214 - GuillaumeGomez:rollup, r=GuillaumeGomez

535b6d3

Rollup of 9 pull requests - Successful merges: #38085, #38123, #38151, #38153, #38158, #38163, #38186, #38189, #38208 - Failed merges:

GuillaumeGomez approved these changes Dec 11, 2016

View reviewed changes

doc: Explain meaning of Result iters and link to factory functions.

16d4b7b

sourcefrog force-pushed the doc-iter branch from c41f5dd to 16d4b7b Compare December 13, 2016 19:29

sourcefrog mentioned this pull request Dec 13, 2016

[rfc] command to squash and merge servo/homu#70

Open

GuillaumeGomez requested changes Dec 14, 2016

View reviewed changes

GuillaumeGomez mentioned this pull request Dec 15, 2016

Rollup of 6 pull requests #38387

Closed

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 15, 2016

Rollup merge of rust-lang#38158 - sourcefrog:doc-iter, r=GuillaumeGomez

5332e93

Explain meaning of Result iters and link to factory functions

GuillaumeGomez approved these changes Dec 16, 2016

View reviewed changes

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 16, 2016

Rollup merge of rust-lang#38158 - sourcefrog:doc-iter, r=GuillaumeGomez

0d4772e

Explain meaning of Result iters and link to factory functions

GuillaumeGomez mentioned this pull request Dec 16, 2016

Rollup of 7 pull requests #38407

Closed

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 18, 2016

Rollup merge of rust-lang#38158 - sourcefrog:doc-iter, r=GuillaumeGomez

5f3b2c7

Explain meaning of Result iters and link to factory functions

GuillaumeGomez mentioned this pull request Dec 18, 2016

Rollup of 9 pull requests #38450

Closed

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Dec 20, 2016

Rollup merge of rust-lang#38158 - sourcefrog:doc-iter, r=GuillaumeGomez

58d58c2

Explain meaning of Result iters and link to factory functions

alexcrichton mentioned this pull request Dec 20, 2016

Rollup of 29 pull requests #38499

Merged

bors merged commit 16d4b7b into rust-lang:master Dec 21, 2016

Explain meaning of Result iters and link to factory functions #38158

Explain meaning of Result iters and link to factory functions #38158

Conversation

sourcefrog commented Dec 4, 2016

rust-highfive commented Dec 4, 2016

bluss commented Dec 4, 2016

bors commented Dec 4, 2016

GuillaumeGomez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Dec 7, 2016

sourcefrog commented Dec 11, 2016

sourcefrog commented Dec 11, 2016

GuillaumeGomez commented Dec 11, 2016

sourcefrog commented Dec 13, 2016

sourcefrog commented Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez Dec 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez Dec 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Dec 15, 2016

bors commented Dec 15, 2016

bors commented Dec 16, 2016

bors commented Dec 16, 2016

GuillaumeGomez commented Dec 16, 2016

bors commented Dec 16, 2016

bors commented Dec 16, 2016

alexcrichton commented Dec 16, 2016 via email

sanxiyn commented Dec 19, 2016

alexcrichton commented Dec 20, 2016

sourcefrog commented Dec 13, 2016 •

edited

Loading

GuillaumeGomez Dec 14, 2016 •

edited

Loading

GuillaumeGomez Dec 15, 2016 •

edited

Loading