-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[ENH] Add rust rendezvous hashing and errors #1508
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
} | ||
|
||
/// Error codes for assignment | ||
#[derive(Error, Debug)] |
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.
This is an example of how to create errors.
HashError, | ||
} | ||
|
||
impl ChromaError for AssignmentError { |
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.
We must always do this, it's not compile time enforced, I can't find a good way to do that right now, but higher levels of code can Box<dyn ChromaError>
this way and rely on codes.
00a3b94
to
cb0e880
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 good! One small comment nit but that's it :)
let mut max_member = None; | ||
|
||
for member in members { | ||
if !iterated { |
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.
Note to myself: we need this because count()
on a rust Iterator
consumes the iterator and size_hint()
explicitly states that iterators don't have to implement it correctly. So there's no way to check the size of members
at the top of the function.
cd4f576
to
cb0e880
Compare
Description of changes
Summarize the changes made by this PR.
Test plan
How are these changes tested?
cargo test
Documentation Changes
None required. Please evaluate the documentation quality of the code itself.