-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
copr: Implement ChunkedVecSet #8988
Conversation
Signed-off-by: Xuanwo <github@xuanwo.io>
|
ChunkedVecSet's implementation looks nearly the same with ChunkedVecEnum, can we share them in some way? |
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
|
/run-all-tests |
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.
Rest LGTM
It looks like we need to implement all the traits before we export them, so I planed to export them in later PRs (after we implement them all) |
Signed-off-by: Xuanwo <github@xuanwo.io>
|
I am not sure. I prefer to merge runnable / testable codes into master. Is it too hard to achieve? |
|
I think implement |
|
My plan is to add Is this plan OK? |
Sounds good to me. So I think in this PR, the two modules |
|
By the way, I think it takes little effort to implement |
|
All code has been implemented locally, just splitted into different PRs, I prefer split |
Alright! |
+ if !self.is_set(idx) { |
Signed-off-by: Xuanwo <github@xuanwo.io>
|
/cc @iosmanthus |
|
I don't think it's appropriate to merge a PR that doesn't compile. PR should be considered as a minimum compliable unit. |
Signed-off-by: Xuanwo <github@xuanwo.io>
|
OK, I will export mod in this PR to make sure all code compliable. |
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
|
/run-all-tests |
Signed-off-by: Xuanwo <github@xuanwo.io>
|
PTAL @zhongzc |
Signed-off-by: Xuanwo <github@xuanwo.io>
|
Filename can be more unified, BTW, can you add copyright info for the newly added files? |
Signed-off-by: Xuanwo <github@xuanwo.io>
If doing so, importing this module |
|
|
Signed-off-by: Xuanwo <github@xuanwo.io>
interesting.. |
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
|
/merge |
|
/run-all-tests |
|
@Xuanwo merge failed. |
|
/run-all-tests |
|
@Xuanwo merge failed. |
|
/merge |
|
/run-all-tests |
What problem does this PR solve?
Implement ChunkedVecSet
Check List
Tests