-
Notifications
You must be signed in to change notification settings - Fork 23
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
Make CommitProcessor another variant of ReceivedMessage. #241
Conversation
…it_processor function
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 1.x-main #241 +/- ##
============================================
- Coverage 90.34% 90.30% -0.04%
============================================
Files 177 177
Lines 31318 31365 +47
============================================
+ Hits 28294 28325 +31
- Misses 3024 3040 +16 ☔ View full report in Codecov by Sentry. |
mls-rs/src/external_client/group.rs
Outdated
#[allow(clippy::large_enum_variant)] | ||
pub enum ExternalReceivedMessage { | ||
/// State update as the result of a successful commit. | ||
pub enum ExternalReceivedMessage<'a, C: ExternalClientConfig> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just food for thought, the 'a nature of this might get in the way of passing this around or storing it places. Maybe it makes more sense for the oneshot function to return simply ExternalReceivedMessage, and the non-oneshot function to return an enum that is either ExternalReceivedMessage or CommitProcessor
ReceivedMessage::Commit(value) => f.write_str(&format!("Commit({value:?})")), | ||
ReceivedMessage::Proposal(value) => f.write_str(&format!("Proposal({value:?})")), | ||
ReceivedMessage::GroupInfo(value) => f.write_str(&format!("GroupInfo({value:?})")), | ||
ReceivedMessage::KeyPackage(value) => f.write_str(&format!("KeyPackage({value:?})")), | ||
ReceivedMessage::Welcome => f.write_str("Welcome"), | ||
ReceivedMessage::CommitProcessor(_) => f.write_str("CommitProcessor"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above for this, might simplify a bit
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT license.