Skip to content

Conversation

@lidavidm
Copy link
Member

@lidavidm lidavidm commented Jun 7, 2021

This also adds the regular case-insensitive count_substring.

@github-actions
Copy link

github-actions bot commented Jun 7, 2021

@pitrou
Copy link
Member

pitrou commented Jun 10, 2021

Can you rebase this @lidavidm ?

@lidavidm
Copy link
Member Author

Rebased, thanks for the reminder.

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

LGTM, just a nit

Copy link
Member

Choose a reason for hiding this comment

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

Nit, but may use offset_bit_width() from type_traits.h
(e.g offset_bit_width(ty->id()) == 64 ? int64() : int32())

@pitrou
Copy link
Member

pitrou commented Jun 10, 2021

Hmm, there are conflicts now...

@lidavidm
Copy link
Member Author

It's because the tests overlap/I forgot to rename & move the test case for ascii_replace_slice (now done).

@pitrou
Copy link
Member

pitrou commented Jun 10, 2021

Feel free to merge if CI green!

@lidavidm
Copy link
Member Author

Thanks for the review!

CI green other than the usual culprits, merging.

@lidavidm lidavidm closed this in 1830d15 Jun 10, 2021
@lidavidm lidavidm deleted the arrow-12952 branch June 10, 2021 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants