Closed akesling closed 4 years ago
Mythra
commented
4 years ago Thanks (again today :wink: )!
This used to need to be mutable due to how we used to wrap the native libsodium libraries (quite hackily I might add). Luckily we haven't had to do that for awhile (since other much nicer interfaces supported the methods we needed). So we really don't need this mutable requirement anymore.
Can you add this to the changelog? That way we can track it when we cut the next release.
akesling
commented
4 years ago Thanks (again today 😉 )!
This used to need to be mutable due to how we used to wrap the native libsodium libraries (quite hackily I might add). Luckily we haven't had to do that for awhile (since other much nicer interfaces supported the methods we needed). So we really don't need this mutable requirement anymore.
Ah yes, vestigial oddities. I'm glad to hear I wasn't missing anything special :).
Can you add this to the changelog? That way we can track it when we cut the next release.
Done. PTAL.
Motivation (required)
What existing problem does the pull request solve?
It's unclear why this was mut, so I may be overlooking something... and please let me know if I am. That being said, the use of a mutable type for what seemed a non-returning constant parameter appeared a code smell. By avoiding unnecessary mutability, we may sidestep a class of accidental errors and hopefully have more robust (and correct) software. Also, I won't have to worry about my key being mutated out from under me between calls :P.
Test Plan (required)
Tests pass and examples run. As this is purely a parameter type change, that should hopefully be sufficient test surface.