-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
epoch: derive Hash #170
epoch: derive Hash #170
Conversation
Hash is required if we want structures like "HashMap" or "BTreeMap" indexed by an `Epoch` Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Thanks! That is definitely useful. I'm not sure why Clippy doesn't like it. Here is the explanation: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq . Any idea how to fix this? Should |
Implement Hash ourselves so it is consistent with PartialEq Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Hello @ChristopherRabotin
https://www.philipdaniels.com/blog/2019/rust-equality-and-ordering/ My mere understanding is that Hash and Eq are closely related. |
Codecov ReportBase: 78.79% // Head: 78.57% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #170 +/- ##
==========================================
- Coverage 78.79% 78.57% -0.22%
==========================================
Files 9 9
Lines 2579 2586 +7
==========================================
Hits 2032 2032
- Misses 547 554 +7
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Perfect, I think that implementing the hasher manually works well (and makes clippy happy). Also, the Epoch hash is based only on the duration since J1900 TAI, which is what the equality checks too, so that's the correct behavior. Thanks! |
Hash is required if we want structures like "HashMap" or "BTreeMap"
indexed by an Epoch
Signed-off-by: Guillaume W. Bres guillaume.bressaix@gmail.com