Improve std::hash docs #41125
Merged
Improve std::hash docs #41125
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Part of #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`
Part of #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
Part of #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
Part of #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
chordowl
commented
Apr 7, 2017
src/libcore/hash/mod.rs
Outdated
| /// assert_eq!(hasher_1.finish(), hasher_2.finish()); | ||
| /// ``` | ||
| /// | ||
| /// [`build_hasher`]: #method.build_hasher |
There was a problem hiding this comment.
This needs to link to #tymethod.build_hasher (I forgot to run linkchecker locally >_>)
|
/cc @rust-lang/docs |
frewsxcv
reviewed
Apr 10, 2017
src/libcore/hash/mod.rs
Outdated
| /// #[derive(Hash)] | ||
| /// struct Rustacean { | ||
| /// name: String, | ||
| /// country: String, |
There was a problem hiding this comment.
We tend to use four spaces for indentation.
src/libcore/hash/mod.rs
Outdated
| /// | ||
| /// If you need more control over how a value is hashed, you need to implement | ||
| /// the `Hash` trait: | ||
| /// If you need more control over how a value is hash, you can of course |
|
I have addressed the broken link as well as the indent and grammar slipups that were brought up. Since I personally have barely any idea on how to act on the issues I outline in the initial comment, and as they weren't on the list on the issue anyway, I've decided not to address them within this PR (unless that is a dealbreaker for someone 😅). |
alexcrichton
added
the
A-docs
Mark-Simulacrum
added
the
S-waiting-on-review
|
This is great @lukaramu, thanks so much for this! 🎉 @bors r+ rollup |
|
📌 Commit 12d7c3d has been approved by |
frewsxcv
added
S-waiting-on-bors
|
⌛ Testing commit 12d7c3d with merge 6874b74... |
frewsxcv
added a commit
to frewsxcv/rust
that referenced
this pull request
Apr 15, 2017
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 retry prioritizing rollup |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
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...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?ShouldHasherget an example for implementing it? There is one in the module documentation, but it only "implements" it viaunimplemented!and I'm not sure what the value of that is.ShouldHasher'swrite_{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