-
Couldn't load subscription status.
- Fork 29
refactor: introducing votor-messages crate #328
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
Conversation
|
I could use feedback on |
01a5522 to
3a4f846
Compare
formatting abi updates sort dependencies from_certificate_type -> new CertificateId -> Certificate votor -> votor-messages
3a4f846 to
c1929d1
Compare
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.
LGTM thanks for cleaning this up!
| fn receive_new_certificates( | ||
| certificate_receiver: &Receiver<(CertificateId, CertificateMessage)>, | ||
| ) -> Result<Vec<(CertificateId, CertificateMessage)>> { | ||
| certificate_receiver: &Receiver<(Certificate, CertificateMessage)>, |
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.
Originally we did this because Certificate contains something CertificateId doesn't have. Now you use Certificate, CertificateMessage should have Certificate.
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.
That said, it's probably easier to just do #299
|
For the future please don't merge PRs before the checks pass |
|
Checks already passed several times (this may not appear because of the rebases I've been making). That's fine though - can wait a bit longer in the future for the final green checks to show up. |
Problem
We'd like to factor out message types employed in votor into a new crate.
Summary of Changes
Perform said refactoring. Addresses #294.