Skip to content

Show type for docs slice Chunks #77938

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

Closed
wants to merge 1 commit into from
Closed

Conversation

pickfire
Copy link
Contributor

No description provided.

@rust-highfive
Copy link
Contributor

r? @sfackler

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 14, 2020
@pickfire
Copy link
Contributor Author

r? @GuillaumeGomez

@GuillaumeGomez
Copy link
Member

Thanks!

r=me once CI pass

@GuillaumeGomez
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 14, 2020

📌 Commit f18d4ff has been approved by GuillaumeGomez

@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 Oct 14, 2020
@GuillaumeGomez
Copy link
Member

On second-thought, I think it kinda deserves the original purpose which is "how do I get this type". I'll let others decide.

@bors: r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 14, 2020
@scottmcm
Copy link
Member

I feel like this should have libs or doc come to a bigger decision about examples on all the adapter types. Most of them just point to the method on Iterator and say "See its documentation for more", which has always seemed fine to me -- following the hyperlink isn't a problem, and is better than copying the documentation since the type is never used in isolation. (And almost never typed out explicitly, in my experience, especially now that -> impl Iterator<Item = T> works.)

If this PR is desirable, it seems like that would imply that there should also be ones for Cloned, Copied, Cycle, Empty, Enumerate, and more.

@pickfire
Copy link
Contributor Author

@GuillaumeGomez I did mentioned these last time, I thought we started adding examples for structs?

@scottmcm Yes, the link to the original function is still there, I initially don't agree with this when I saw @GuillaumeGomez sending how to create the struct, but then I found @jyn514 sending suggestion on giving explicit type to the struct.

Then I find it useful, since now we know the explicit struct type without digging much given the examples, like Chunks<char> here, it may not be as straightforward for a beginner to read the generics signature but this helps me understand a lot.

@jyn514
Copy link
Member

jyn514 commented Oct 15, 2020

I think this is better than before - maybe not by a lot, but it makes it more clear what the types are. Iterators are pretty 'magic' when you're new to the language and I think this makes them less mysterious.

If this PR is desirable, it seems like that would imply that there should also be ones for Cloned, Copied, Cycle, Empty, Enumerate, and more.

Sure, that sounds good to me ;)

@jyn514 jyn514 added A-iterators Area: Iterators A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools I-needs-decision Issue: In need of a decision. labels Oct 15, 2020
@GuillaumeGomez
Copy link
Member

@GuillaumeGomez I did mentioned these last time, I thought we started adding examples for structs?

That wasn't my complain. They should have examples. My point was just that maybe there was too much information. Is it really necessary to add type information?

@pickfire
Copy link
Contributor Author

That wasn't my complain. They should have examples. My point was just that maybe there was too much information. Is it really necessary to add type information?

I find it good such that they can easily understand the additional type system rather than going to read generics and understand it, concrete types certainly is easier to comprehend.

@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 6, 2020
@crlf0710
Copy link
Member

crlf0710 commented Nov 6, 2020

Triage: It seems this is waiting on team decision? Let me mark it as such.

@pickfire
Copy link
Contributor Author

pickfire commented Nov 6, 2020

@GuillaumeGomez Similar patch #78776

@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Nov 28, 2020
@crlf0710 crlf0710 removed the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Dec 18, 2020
@crlf0710 crlf0710 added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Dec 18, 2020
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Jan 15, 2021
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Mar 5, 2021
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Apr 2, 2021
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Apr 23, 2021
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels May 14, 2021
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Jun 5, 2021
@apiraino
Copy link
Contributor

@crlf0710 any team in particular you are asking for a review? what's the context here?

cc @pickfire

@pickfire
Copy link
Contributor Author

Doc team but I think they are busy.

@jyn514
Copy link
Member

jyn514 commented Jun 24, 2021

There is no doc team. The best reviewer for this is probably someone from T-libs.

@apiraino apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 24, 2021
@m-ou-se m-ou-se assigned m-ou-se and unassigned GuillaumeGomez Jul 1, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Jul 1, 2021

Agreed with what @scottmcm said. None of the other iterator adapters have examples in the type documentation. The example on this type doesn't really add anything over the link to slice::chunks. The slice::chunks documentation already clearly shows the signature including Chunks in the return type, and has a more complete example. I'd like to avoid duplicating documentation and examples over the adapter functions and types. So let's just remove this example and just point people directly to slice::chunks for examples and documentation.

@pickfire
Copy link
Contributor Author

pickfire commented Jul 1, 2021

Should I just close this?

@crlf0710 crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). I-needs-decision Issue: In need of a decision. labels Jul 17, 2021
@JohnCSimon JohnCSimon added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. labels Jul 31, 2021
@JohnCSimon
Copy link
Member

Triage:
@pickfire what would you like to do with this PR?

@pickfire
Copy link
Contributor Author

pickfire commented Aug 2, 2021

I will close this.

@pickfire pickfire closed this Aug 2, 2021
@pickfire pickfire deleted the patch-1 branch August 2, 2021 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-iterators Area: Iterators S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.