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

Trait documentation clarifications #33815

Merged
merged 21 commits into from
May 27, 2016
Merged

Trait documentation clarifications #33815

merged 21 commits into from
May 27, 2016

Conversation

carols10cents
Copy link
Member

Hi! I've felt a bit of friction lately in figuring out how to write custom implementations of the deriveable traits, so I decided to add to the docs :)

The docs for Copy are already excellent-- clear, useful sections that I only reordered a bit-- they're now:

  • General explanation
  • When can my type be Copy?
  • When can my type not be Copy?
  • When should my type be Copy?
  • Derivable
  • How can I implement Copy?

I didn't add all these sections for all the traits, but I did make sure all the derivable traits had a consistent "Derivable" section that explained what the derived implementation does and a "How can I implement" section that has an example.

Please check me for correctness-- I tried to do research to make sure I was saying accurate things but I'm still learning! ❤️ I'd also love suggestions on information to add that is still missing-- I think these traits are important and deserve to have awesome docs!

I think these just got out of sync, but both use a lexicographic
ordering.

Relevant commits in the history of these explanations:
* 8b81f76 on 2015-06-30
* e22770b on 2016-02-09
This matches the other subsections.
The new order puts all the "when" questions together and puts the "how"
question with the "derivable" section. So you have to scroll past (and
hopefully read) the can/cannot/should caveats and guidelines to get to
the information about how to actually go about doing it once you've
determined that you can and should, with derivable information first so
that you can just use the derived implementation if that applies.

Previous order:

* General explanation
* When can my type be `Copy`?
* How can I implement `Copy`?
* When can my type _not_ be `Copy`?
* When should my type be `Copy`?
* Derivable

New order:

* General explanation
* When can my type be `Copy`?
* When can my type _not_ be `Copy`?
* When should my type be `Copy`?
* Derivable
* How can I implement `Copy`?
Including an example of a custom implementation. I put this expanded
section after the `Derivable` section to encourage use of that first.
Building on the example in PartialEq.
A bit of duplication from the module documentation, but simplified
to be closer to being trivially copy-paste-able.
Similar to the `Ord` examples but calling out that it can be defined
using `cmp` from `Ord` or using `partial_cmp` in a situation that
demands that.
Add explicit "Derivable" and "How can I implement `Default`" sections.
Copied relevant sections from the module-level documentation, but also
linked to there-- it has a more comprehensive narrative with examples
that show implementation AND use. Decided to just put implementation
example in the trait documentation.
/// A common trait for cloning an object.
/// A common trait for cloning an object. Differs from `Copy` in that you can
/// define `Clone` to run arbitrary code, while you are not allowed to override
/// the implementation of `Copy` that only does a `memcpy`.
Copy link
Member

Choose a reason for hiding this comment

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

Okay, GitHub is being weird, and when I refreshed, my comments were gone, so trying again. Sorry if this is a duplicate.

First, this summary line is a bit long. We try to keep the very first line a sentence, and while what was here was a bit... sparse, the summary line should still be shorter than this.

Second, the Copy/Clone distinction isn't really about overriding, exactly. Or rather, to me, the not-overriding is the implementation rather than the important bit. The important bit is that Copy is extremely inexpensive and is implicit. Clone may or may not be expensive, and is always explicit. The mechanism to ensure this is to not let you implement the details of a Copy.

I also have small concerns about putting stuff about Copy's semantics right here in the summary; while they won't really be changing.... idk. Hm.

Copy link
Member Author

Choose a reason for hiding this comment

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

I love your explanation of the distinction between Clone and Copy, working that in.

@steveklabnik
Copy link
Member

steveklabnik commented May 23, 2016

This is great, thanks! I hope github's weirdness today didn't lose the rest of my comments...... oh hey there, they just appeared.

@carols10cents
Copy link
Member Author

Thank you for the review!! I think I have all the comments; I'll cross-check between here and email. Addressing your awesome suggestions now!!! ❤️

I had already copied the implementation example in a previous commit;
this copies the explanation and usage examples to the general trait
description.
@carols10cents
Copy link
Member Author

Whew! Ok, I think I've got everything and make check-docs passing. ❤️

@steveklabnik
Copy link
Member

Let's :shipit:

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented May 24, 2016

📌 Commit 1e809f5 has been approved by steveklabnik

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 27, 2016
…larifications, r=steveklabnik

Trait documentation clarifications

Hi! I've felt a bit of friction lately in figuring out how to write custom implementations of the `derive`able traits, so I decided to add to the docs :)

The docs for `Copy` are already excellent-- clear, useful sections that I only reordered a bit-- they're now:

* General explanation
* When can my type be `Copy`?
* When can my type _not_ be `Copy`?
* When should my type be `Copy`?
* Derivable
* How can I implement `Copy`?

I didn't add all these sections for all the traits, but I did make sure all the derivable traits had a consistent "Derivable" section that explained what the derived implementation does and a "How can I implement" section that has an example.

Please check me for correctness-- I tried to do research to make sure I was saying accurate things but I'm still learning! ❤️ I'd also love suggestions on information to add that is still missing-- I think these traits are important and deserve to have awesome docs!
bors added a commit that referenced this pull request May 27, 2016
Rollup of 10 pull requests

- Successful merges: #33753, #33815, #33829, #33858, #33865, #33866, #33870, #33874, #33891, #33898
- Failed merges:
@bors bors merged commit 1e809f5 into rust-lang:master May 27, 2016
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.

3 participants