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

API Docs: hash #29357

Closed
steveklabnik opened this Issue Oct 26, 2015 · 9 comments

Comments

Projects
None yet
5 participants
@steveklabnik
Member

steveklabnik commented Oct 26, 2015

Part of #29329

http://doc.rust-lang.org/std/hash/

Here's what needs to be done to close out this issue:

  • BuildHasherDefault needs a stronger summary/explanation distinction. "This structure is zero-sized and does not need construction." is weird.
  • BuildHasher also needs a stronger summary/explanation distinction. It also needs a better explanation of why it exists with an example.
  • Hash needs general improvement, it is the main bit of this module, but its docs feel tossed together and have few examples or coherence.
  • Hasher needs more than just a summary line; it needs explanation and examples.
@adrianbrink

This comment has been minimized.

adrianbrink commented Jul 28, 2016

What kind of further documentation are you looking for here? @steveklabnik

@steveklabnik

This comment has been minimized.

@piordanov

This comment has been minimized.

piordanov commented Feb 26, 2017

There seem to be a lengthy example of BuildHasherDefault added in this commit (d409fc3)

Also for BuildHasher and Hash.html there is a healthy amount of documentation as of now. Hasher seems to be really outdated (still lists several Implementors without mentioning they have been deprecated), which I'm looking into right now.

@brson

This comment has been minimized.

Contributor

brson commented Feb 26, 2017

Thanks @piordanov ! Looks like all that's left is documenting Hasher.

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Mar 8, 2017

I am happy to mentor anyone who wants to tackle this issue.

@adrianbrink

This comment has been minimized.

adrianbrink commented Mar 10, 2017

@steveklabnik I volunteer as a tribute :-)

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Mar 10, 2017

@adrianbrink wonderful! If you need any help, please just let me know, as I said above, happy to help you help me 😄 (We can do it here or ping me on IRC)

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Mar 24, 2017

@adrianbrink I have added more specifics in the main issue above

@lukaramu

This comment has been minimized.

Contributor

lukaramu commented Apr 5, 2017

I'm gonna tackle this one this weekend! (if that's okay with you, @adrianbrink)

lukaramu added a commit to lukaramu/rust that referenced this issue Apr 6, 2017

rephrased std:#️⃣:BuildHasherDefault docs
Part of rust-lang#29357.
* split summary and explanation more clearly
* added link to nomicon for "zero-sized"
* "does not need construction" -> say how it can be created, and that it
  doesn't need to be done with `HashMap` or `HashSet`

lukaramu added a commit to lukaramu/rust that referenced this issue Apr 6, 2017

improved std:#️⃣:BuildHasher docs
Part of rust-lang#29357.
* split summary and explanation more clearly, while expanding the
  explanation to make the reason for `BuildHasher` existing more clear
* added an example illustrating that `Hasher`s created by one `BuildHasher`
  should be identical
* added links
* repeated the fact that hashers produced should be identical in
  `build_hasher`s method docs

lukaramu added a commit to lukaramu/rust that referenced this issue Apr 6, 2017

improved std:#️⃣:Hash docs
Part of rust-lang#29357.
* merged "Derivable" and "How can I implement `Hash`?" sections into one
  "Implementing `Hash`" section to aid coherency
* added an example for `#[derive(Hash)]`
* moved part about relation between `Hash` and `Eq` into a new "`Hash` and
  `Eq`" section; changed wording to be more consistent with `HashMap` and
  `HashSet`'s
* explicitly mentioned `#[derive(PartialEq, Eq, Hash)]` in the new section
* changed method summaries for consistency, adding links and examples

lukaramu added a commit to lukaramu/rust that referenced this issue Apr 6, 2017

improved std:#️⃣:Hasher docs
Part of rust-lang#29357.
* rephrased summary sentences to be less redundant
* expanded top-level docs, adding a usage example and links to relevant
  methods (`finish`, `write` etc) as well as `Hash`
* added examples to the `finish` and `write` methods

lukaramu added a commit to lukaramu/rust that referenced this issue Apr 6, 2017

improved std:#️⃣:Hasher docs
Part of rust-lang#29357.
* rephrased summary sentences to be less redundant
* expanded top-level docs, adding a usage example and links to relevant
  methods (`finish`, `write` etc) as well as `Hash`
* added examples to the `finish` and `write` methods

@lukaramu lukaramu referenced this issue Apr 6, 2017

Merged

Improve std::hash docs #41125

4 of 4 tasks complete

bors added a commit that referenced this issue Apr 15, 2017

Auto merge of #41125 - lukaramu:std-hash-docs, r=frewsxcv
Improve std::hash docs

Fixes #29357.

For details on what exactly I've done, see the commit descriptions.

There are some things I'm not sure about, but would like to address before merging this so the issue can be closed; any feedback on these points would really be appriciated:
* [x] ~I didn't touch the module level docs at all. On the one hand, I think they could use a short overview over the module; on the other hand, the module really isn't that big and I don't know if I could really do anything beyond just duplicating the type's summaries...~
* [x] ~I feel like the module-level examples are quite long-winded and not to the point, but I couldn't really think of anything better. Any ideas?~
* [x] ~Should `Hasher` get an example for implementing it? There is one in the module documentation, but it only "implements" it via `unimplemented!` and I'm not sure what the value of that is.~
* [x] ~Should `Hasher`'s `write_{int}` methods get examples?~

If there's anything else you'd like to see in std::hash's docs, please let me know!

r? @rust-lang/docs

frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 15, 2017

Rollup merge of rust-lang#41125 - lukaramu:std-hash-docs, r=frewsxcv
Improve std::hash docs

Fixes rust-lang#29357.

For details on what exactly I've done, see the commit descriptions.

There are some things I'm not sure about, but would like to address before merging this so the issue can be closed; any feedback on these points would really be appriciated:
* [x] ~I didn't touch the module level docs at all. On the one hand, I think they could use a short overview over the module; on the other hand, the module really isn't that big and I don't know if I could really do anything beyond just duplicating the type's summaries...~
* [x] ~I feel like the module-level examples are quite long-winded and not to the point, but I couldn't really think of anything better. Any ideas?~
* [x] ~Should `Hasher` get an example for implementing it? There is one in the module documentation, but it only "implements" it via `unimplemented!` and I'm not sure what the value of that is.~
* [x] ~Should `Hasher`'s `write_{int}` methods get examples?~

If there's anything else you'd like to see in std::hash's docs, please let me know!

r? @rust-lang/docs

@bors bors closed this in #41125 Apr 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment