-
Notifications
You must be signed in to change notification settings - Fork 3
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
use simplfied Bitmap #120
use simplfied Bitmap #120
Conversation
Replace `bitmaps::Bitmap` with a simplified bitmap implementation for the 256 wide use case needed by boreal. This does a few things: * Provide a simplified interface for Bitmap based on what's used by boreal * Replaces a bunch of casts & slice indexing that could cause future issues with compile-time validation (by moving from indexing into the Bitmap by usize to u8) * Removes a MPL2 licensed dependency (which can be incompatible with
note: the coverage task is currently failing due to issues addressed in #122. |
Sorry about all the CI issues. I wouldn't mind removing a dependency, especially as we only use the 256 sized bitmap, so making a simple, custom impl makes sense. I'll try to review this before the end of the week |
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.
Thanks for this contribution, I have a few comments but otherwise lgtm. I'll probably add a few additional tests afterwards but it's fine for this PR
@vthib I've applied your suggestions. |
@demoray approved, but needs a rebase, probably after the merges of the nightly fixes. |
Replace
bitmaps::Bitmap
with a simplified bitmap implementation for the 256 wide use case needed by boreal.This does a few things: