Skip to content

Conversation

@boazberman
Copy link
Contributor

Which issue does this PR close?

Ref apache/datafusion#342
Ref apache/datafusion#240

Rationale for this change

Described in the DataFusion issue. To make BooleanBufferBuilder useable as a bit vector within DataFusion code I needed this api exposed as well.

What changes are included in this PR?

Are there any user-facing changes?

@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2021

Codecov Report

Merging #693 (eb2ff69) into master (1a5c26b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #693   +/-   ##
=======================================
  Coverage   82.44%   82.44%           
=======================================
  Files         168      168           
  Lines       47347    47365   +18     
=======================================
+ Hits        39033    39052   +19     
+ Misses       8314     8313    -1     
Impacted Files Coverage Δ
arrow/src/array/builder.rs 86.13% <100.00%> (+0.15%) ⬆️
parquet/src/encodings/encoding.rs 94.67% <0.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a5c26b...eb2ff69. Read the comment docs.

Copy link
Contributor

@Dandandan Dandandan left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks good to me. The tests look good as well

cc @jorgecarleitao @nevi-me

@boazberman
Copy link
Contributor Author

How many approvers does one need to get a pr merged?

@Dandandan Dandandan merged commit 0b7ac79 into apache:master Aug 15, 2021
@Dandandan
Copy link
Contributor

How many approvers does one need to get a pr merged?

We don't have a minimum reviewers requirement. The PRs will be merged after some common sense judgement, based on the complexity of the changes and giving other reviewers time to review and comment.

As the change is small and not really controversial I just merged it
Thanks again! 🎉

@alamb
Copy link
Contributor

alamb commented Aug 16, 2021

I plan to backport this change to 5.3.0 (eta in about two weeks time)

alamb pushed a commit that referenced this pull request Aug 19, 2021
* Add get_bit to BooleanBufferBuilder

* fix clippy
@alamb
Copy link
Contributor

alamb commented Aug 19, 2021

Backport in #699. I plan to make 5.3.0 release candidate around Aug 27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants