Skip to content
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

optimize roaring_bitmap_statistics (breaking change) #624

Merged

Conversation

AviAvni
Copy link
Contributor

@AviAvni AviAvni commented May 5, 2024

implement #619

@AviAvni
Copy link
Contributor Author

AviAvni commented May 8, 2024

@Dr-Emann ping

@lemire
Copy link
Member

lemire commented May 8, 2024

Looks good to me. It is a breaking change, but one that should be fine.

@lemire
Copy link
Member

lemire commented May 8, 2024

Feedback invited!!!

Copy link
Member

@Dr-Emann Dr-Emann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lemire do you have a place to keep track of "things to change next major version"? Removing the sum field entirely seems like a good thing to do next major version bump.

@lemire
Copy link
Member

lemire commented May 12, 2024

do you have a place to keep track of "things to change next major version"? Removing the sum field entirely seems like a good thing to do next major version bump.

I would propose just doing a major version now, and indicating this breaking change.

If there is general agreement, I would just do it as simply as that.

I am not aware of any other breaking change, are you?

I mean, we added many things, but it would be the first time we remove some functionality in a long time.

@lemire lemire changed the title optimize roaring_bitmap_statistics optimize roaring_bitmap_statistics (breaking change) May 13, 2024
@lemire
Copy link
Member

lemire commented May 13, 2024

I am going to merge and issue a major version.

@lemire lemire merged commit 659b4a8 into RoaringBitmap:master May 13, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants