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

Improved docstrings for free::chain #634

Merged
merged 1 commit into from Aug 25, 2022

Conversation

JoelMon
Copy link
Contributor

@JoelMon JoelMon commented Jul 11, 2022

Improved the description of the function and added example.

src/free.rs Outdated
@@ -128,16 +128,25 @@ pub fn zip<I, J>(i: I, j: J) -> Zip<I::IntoIter, J::IntoIter>
i.into_iter().zip(j)
}

/// Create an iterator that first iterates `i` and then `j`.
/// `chain()` returns the result of combining the _first_
Copy link
Contributor

Choose a reason for hiding this comment

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

fix: As in #633 (comment), this is not just about https://doc.rust-lang.org/nightly/std/primitive.array.htmls, so shouldn't be written this way.

@JoelMon JoelMon marked this pull request as draft Jul 11, 2022
@JoelMon
Copy link
Contributor Author

JoelMon commented Jul 11, 2022

@scottmcm: std::iter::chain this means that this function was also stabilized in std, right?

Copy link
Member

@phimuemue phimuemue left a comment

Choose a reason for hiding this comment

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

Hi there, thanks for this - improvements to documentation is always good!

It would be nice if you could incorporate the suggestions (or refute them with good reason), and sqash your changes into a single commit.

src/free.rs Outdated Show resolved Hide resolved
src/free.rs Outdated Show resolved Hide resolved
src/free.rs Outdated Show resolved Hide resolved
@JoelMon JoelMon force-pushed the chain-docstring branch 2 times, most recently from 94326f3 to 860f239 Compare Aug 25, 2022
@JoelMon
Copy link
Contributor Author

JoelMon commented Aug 25, 2022

I think I finally got this git squashing, force pushing stuff down. It's about time. 😆

@JoelMon JoelMon marked this pull request as ready for review Aug 25, 2022
@JoelMon JoelMon requested review from phimuemue and scottmcm and removed request for phimuemue and scottmcm Aug 25, 2022
Copy link
Member

@phimuemue phimuemue left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. Could you have a second look there? With the suggested changes, I think we're good to go.

src/free.rs Outdated Show resolved Hide resolved
src/free.rs Outdated Show resolved Hide resolved
@JoelMon
Copy link
Contributor Author

JoelMon commented Aug 25, 2022

@phimuemue, would you be able to take a look at #633 as well?

@JoelMon JoelMon requested a review from phimuemue Aug 25, 2022
@phimuemue
Copy link
Member

Thanks a bunch!

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 25, 2022

Build succeeded:

@bors bors bot merged commit 5ae7bb5 into rust-itertools:master Aug 25, 2022
4 checks passed
@JoelMon JoelMon deleted the chain-docstring branch Aug 25, 2022
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.

None yet

3 participants