-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat: recon-over-http #168
Conversation
0b944ab
to
bbb24ca
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.
Looks great, just two small changes to make the API a bit clearer. No need to re-review
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.
Added some comments, just some feedback on idiomatic Rust nothing significant. Feel free to merge once addressed
recon/src/recon/tests.rs
Outdated
@@ -544,7 +545,7 @@ async fn response_is_synchronized() { | |||
let response = a.process_messages(&response.messages).await.unwrap(); | |||
assert!(!response.is_synchronized); | |||
let response = x.process_messages(&response.messages).await.unwrap(); | |||
assert!(!response.is_synchronized); | |||
assert!(response.is_synchronized); |
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.
Why did this test invert?
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.
Can you update the test comments if this is expected?
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.
The is_synchronized
logic changed so that it can detect synchronization one round earlier. Now, we don't have to wait for a round where nothing is exchanged so detect that we're done. We're synchronized as soon as the ranges match.
5cb5e4e
to
75a1843
Compare
75a1843
to
ab2ead2
Compare
No description provided.