Skip to content
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

Fix documentation for Iterator::count(). #66094

Merged
merged 3 commits into from
Nov 13, 2019
Merged

Fix documentation for Iterator::count(). #66094

merged 3 commits into from
Nov 13, 2019

Conversation

ArturKovacs
Copy link
Contributor

The documentation of std::core::Iterator::count() stated that the number returned is the number of times next is called on the iterator. However this is not true as the number of times next is called is exactly one plus the number returned by count().

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cramertj (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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 4, 2019
@@ -202,8 +202,9 @@ pub trait Iterator {
/// Consumes the iterator, counting the number of iterations and returning it.
///
/// This method will evaluate the iterator until its [`next`] returns
/// [`None`]. Once [`None`] is encountered, `count()` returns the number of
/// times it called [`next`].
/// [`None`]. Once [`None`] is encountered, `count()` returns one less than the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like a confusing way to state things. Perhaps something like this would be better: "count() will call [next()] until [None] is encountered, returning the number of times it saw Some(_)."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's a good point. Just replacing count() to "This method" at the beginning feels aesthetically more pleasing. Furthermore using a hyperlink reference to Some is probably beneficial. So may I suggest: "This method will call [next] repeatedly until [None] is encountered, returning the number of times it saw [Some]."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds great! <3

@cramertj
Copy link
Member

cramertj commented Nov 5, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Nov 5, 2019

📌 Commit 23be25c has been approved by cramertj

@JohnCSimon
Copy link
Member

Ping from triage:
@cramertj is this PR ready to merge?
cc: @ArturKovacs
Thanks!

@ArturKovacs
Copy link
Contributor Author

As far as I understand this is ready to be merged, so pinging @cramertj in case he missed the previous ping.

@Dylan-DPC-zz
Copy link

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 13, 2019

📌 Commit 23be25c has been approved by Dylan-DPC

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 13, 2019
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 13, 2019
Fix documentation for `Iterator::count()`.

The documentation of std::core::Iterator::count() stated that the number returned is the number of times `next` is called on the iterator. However this is not true as the number of times `next` is called is exactly one plus the number returned by `count()`.
bors added a commit that referenced this pull request Nov 13, 2019
Rollup of 13 pull requests

Successful merges:

 - #65932 (download .tar.xz if python3 is used)
 - #66074 ([mir-opt] Turn on the `ConstProp` pass by default)
 - #66094 (Fix documentation for `Iterator::count()`.)
 - #66166 (rename cfg(rustdoc) into cfg(doc))
 - #66227 (docs: Fix link to BufWriter::flush)
 - #66292 (add Result::map_or)
 - #66297 (Add a callback that allows compiler consumers to override queries.)
 - #66317 (Use a relative bindir for rustdoc to find rustc)
 - #66330 (Improve non-exhaustiveness handling in usefulness checking)
 - #66331 (Add some tests for fixed ICEs)
 - #66334 (Move Session fields to CrateStore)
 - #66335 (Move self-profile infrastructure to data structures)
 - #66337 (Remove dead code for encoding/decoding lint IDs)

Failed merges:

r? @ghost
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 13, 2019
Fix documentation for `Iterator::count()`.

The documentation of std::core::Iterator::count() stated that the number returned is the number of times `next` is called on the iterator. However this is not true as the number of times `next` is called is exactly one plus the number returned by `count()`.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 13, 2019
Fix documentation for `Iterator::count()`.

The documentation of std::core::Iterator::count() stated that the number returned is the number of times `next` is called on the iterator. However this is not true as the number of times `next` is called is exactly one plus the number returned by `count()`.
bors added a commit that referenced this pull request Nov 13, 2019
Rollup of 14 pull requests

Successful merges:

 - #65932 (download .tar.xz if python3 is used)
 - #66094 (Fix documentation for `Iterator::count()`.)
 - #66166 (rename cfg(rustdoc) into cfg(doc))
 - #66186 (Add long error explanation for E0623)
 - #66227 (docs: Fix link to BufWriter::flush)
 - #66248 (add raw ptr variant of UnsafeCell::get)
 - #66292 (add Result::map_or)
 - #66297 (Add a callback that allows compiler consumers to override queries.)
 - #66317 (Use a relative bindir for rustdoc to find rustc)
 - #66330 (Improve non-exhaustiveness handling in usefulness checking)
 - #66331 (Add some tests for fixed ICEs)
 - #66334 (Move Session fields to CrateStore)
 - #66335 (Move self-profile infrastructure to data structures)
 - #66337 (Remove dead code for encoding/decoding lint IDs)

Failed merges:

r? @ghost
@bors bors merged commit 23be25c into rust-lang:master Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants