Remove dependency on openmls test-utils #1218
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
insipx
changed the title
insipx
force-pushed
the
insipx/cargo-audit
branch
2 times, most recently
from
c6e75c9 to
e4aec8e
Compare
neekolas
reviewed
Nov 5, 2024
neekolas
approved these changes
Nov 5, 2024
There was a problem hiding this comment.
This is great work. The test coverage that is there gives me good confidence this isn't going to break something.
If we want to be really sure, an easy smoke test would be to create a new identity using the CLI on main and upgrading to this version (since we use store the installation keys in the DB). Then we can send a message to another CLI client that is back on main.
insipx
force-pushed
the
insipx/cargo-audit
branch
from
4fdb508 to
685c983
Compare
insipx
force-pushed
the
insipx/cargo-audit
branch
from
685c983 to
88ea048
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@neekolas or @richardhuaaa would appreciate your review on this one if possible. This touches some sensitive code around our installation keys. We were using
ed25519_dalek::SigningKeyin a sortof roundabout way, so I formalized intoxmtp_cryptography::basic_credential. Then removed some clones to remove a dependency ontest-utilswrote some tests to triple-check
basic_credentialis OK, but think this should simplify & centralize installation key operations a bitThis allows us to pass all RUSTSEC advisories. Removing test utils feature also removes a bunch of unused crates in the crate graph for the production binary/ WASM
Part of #1192