-
Notifications
You must be signed in to change notification settings - Fork 91
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
Remove unnecessary lifetimes of CRC digest #129
Remove unnecessary lifetimes of CRC digest #129
Conversation
The CRC digest lifetime was too strict for using it in a method and it can be elided.
✅ Deploy Preview for cute-starship-2d9c9b canceled.
|
CC @huntc - do you know if these lifetimes were chosen intentionally? |
Also, I need to figure out if removing a lifetime like this is a breaking API change. If yes, it might need to wait for #128, but I don't think it is? |
I would assume that removing a lifetime like this is not breaking because That's just my thought though and I'm not 100% sure 😅 |
I've just pulled down the latest commit on main, updated my toolchain to the latest ( All I see that clippy recommends is the following:
The explicit lifetime for Regarding the lifetimes on |
Should we progress this pull request? |
Thank you all! |
The CRC digest lifetime is set to the same as the provided buffer lifetime which means that the CRC has to live as long as the buffer which I don't think is necessary as the
finalize()
method is called on the digest. This strictness of the lifetime meant that I couldn't create aCrc
in the local scope i.e. a method and then just return the bytes (see the new unit test for an example of what I mean).Clippy suggests this change and to remove a few other lifetimes too so I think this should be a low risk change 🙂
Thanks for the great crate!