feat(crc): add crc hash fn #1136
Conversation
src/stdlib/crc32.rs
Outdated
| fn parameters(&self) -> &'static [Parameter] { | ||
| &[Parameter { | ||
| keyword: "value", | ||
| kind: kind::ANY, |
There was a problem hiding this comment.
Hm I noticed this in other hash function. I don't think ANY is ideal but we can leave it here for consistency.
|
Awesome @pront. I have created a PR on vector to document this function |
src/stdlib/crc32.rs
Outdated
There was a problem hiding this comment.
There are many kinds of CRC functions, even many kinds of 32-bit CRC functions. Instead of adding a separate function for each, would it make sense to instead have a single crc function with some "algorithm" parameter like decrypt, encrypt, and hmac use?
There was a problem hiding this comment.
Good point. @ivor11 should be trivial to modify this PR to do the above. Let me know if this makes sense.
There was a problem hiding this comment.
Awesome, thanks. I will take a look at the updated code.
| use crate::value; | ||
| use crc::Crc as CrcInstance; | ||
|
|
||
| fn crc(value: Value, algorithm: Value) -> Resolved { |
There was a problem hiding this comment.
💭 This is one of those (rare) cases where a #[proc_macro] can significantly reduce the boilerplate. We could iterate over valid_algorithms and keep pushing:
$algo => CrcInstance::<u8>::new(&crc::$algo)
.checksum(&value)
.to_string(),
But this is just my observation. Feel free to ignore this comment.
|
This also need a changelog to communicate this new feature to the users. |
|
@pront my bad, I've added the changelog file. Thanks for the thorough review |
Summary
Add crc hash fn with an algorithm parameter
default algorithm used - CRC_32_ISO_HDLC
Change Type
Is this a breaking change?
How did you test this PR?
Does this PR include user facing changes?
our guidelines.
Checklist
run
dd-rust-license-tool writeand commit the changes. More details here.References
#997