feat: add digest-traits feature to implement digest:Update for all hashers#405
feat: add digest-traits feature to implement digest:Update for all hashers#405vmx merged 3 commits intomultiformats:masterfrom
digest-traits feature to implement digest:Update for all hashers#405Conversation
vmx
left a comment
There was a problem hiding this comment.
Thanks for the PR, the changes look good. I'd like to have the upgrade to digest 0.11 in a separate commit, so I just went ahead and did it myself at #406 (a review would be welcome, we are stretched thin on reviewers).
In regards to your change. I don't think we need to feature gate this. Making digest a non-optional shouldn't causes any problems.
Do I understand correctly that the digest-io test is testing that digest::Update is actually implemented? If yes, please add a comment that makes that clearer (at a first look was wondering why there are no tests for it).
|
@vmx Thanks for your quick feedback! If we don't feature-gate this, should I go a step further to implement |
|
@hanabi1224 Good point. Implementing it directly in |
|
@vmx PR updated. |
e2ba60e to
1d3a27f
Compare
| $( | ||
| let expected = hex::decode($expect).unwrap(); | ||
| // Mutlihash enum member, Multihash code, input, Multihash as hex | ||
| {$( $alg:ty, $code:expr, $data:expr, $expect:expr; )*} => { |
There was a problem hiding this comment.
(The original 3-space indent seems to be inconsistent with the rest of the crate)
1d3a27f to
18cc654
Compare
vmx
left a comment
There was a problem hiding this comment.
Thanks for trying different things.
Also your own comment on the whitespace was really useful. Overall, great work!
|
@hanabi1224 Do you plan any other changes atm? I'm asking as I would release a |
|
Hey @vmx Thanks for the quick review! I don't have more changes to propose atm. |
|
The 0.2 release is out! |
This PR
digest-traitsfeature to implementdigest::Updatefor all hashersdigest::Updateis required bydigest_io::IoWrapperwhich is useful for streaming input to a hasher.e.g.