Skip to content

add default docstrings for String and AtomicBool #36364

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

Conversation

matthew-piziak
Copy link
Contributor

@matthew-piziak matthew-piziak commented Sep 9, 2016

Part of #36265.

These are all the instances of Default in this repository that are of the form impl Default for X. The default for i32, for example, is not of this form.

r? @GuillaumeGomez

@matthew-piziak
Copy link
Contributor Author

r? @GuillaumeGomez

@rust-highfive rust-highfive assigned GuillaumeGomez and unassigned nrc Sep 9, 2016
@frewsxcv
Copy link
Member

frewsxcv commented Sep 9, 2016

Relevant pull request, not sure if it's relevant anymore though #23271 . I'm still personally in favor of being explicit about what the default implementation does.

@matthew-piziak
Copy link
Contributor Author

@frewsxcv Oops, didn't see that, thank you. Also, the reason my grep was so sparse was because I didn't catch type-parameterized impls. I'm inclined to close this PR. +1?

@frewsxcv
Copy link
Member

frewsxcv commented Sep 9, 2016

I think this can stay open. #23271 didn't merge, and I don't have any intention to reopen it.

@matthew-piziak
Copy link
Contributor Author

@frewsxcv Okay, sounds good. I would have done this differently had I known what I do now, but it looks like these changes still constitute an improvement.

fn default() -> Generics {
/// Create a empty `Generics` with no lifetimes, type parameters,
/// where-clause, or span.
fn default() -> Generics {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is useful to write an explanation for an inner type.

@matthew-piziak matthew-piziak force-pushed the default-docs branch 2 times, most recently from de1550d to cec3335 Compare September 12, 2016 15:24
@matthew-piziak matthew-piziak changed the title add default docstrings for String, AtomicBool, and Generics add default docstrings for String and AtomicBool Sep 12, 2016
Part of rust-lang#36265.

These are all the instances of `Default` in this repository that are of the form `impl Default for X`. The default for `i32`, for example, is not of this form.

@GuillaumeGomez
@bors
Copy link
Collaborator

bors commented Sep 14, 2016

☔ The latest upstream changes (presumably #36472) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez
Copy link
Member

It seems it has already been done.

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.

5 participants