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

Rename the string kernel to concat_elements. #1752

Merged
merged 1 commit into from
May 26, 2022

Conversation

HaoYang670
Copy link
Contributor

Signed-off-by: remzi 13716567376yh@gmail.com

Which issue does this PR close?

Closes #1747.

Rationale for this change

Please see the issue.

Are there any user-facing changes?

Yes!

Signed-off-by: remzi <13716567376yh@gmail.com>
@github-actions github-actions bot added the arrow Changes to the arrow crate label May 26, 2022
Copy link
Contributor

@tustvold tustvold left a comment

Choose a reason for hiding this comment

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

Some of your finest work 😆

@tustvold
Copy link
Contributor

tustvold commented May 26, 2022

Perhaps we should rename the function string_concat to concat_elements_string to be consistent with other modules?

This would allow us to in future add a concat_elements that accepts any Array or something, or a concat_elements_binary_array, or something...

@alamb
Copy link
Contributor

alamb commented May 26, 2022

Perhaps we should rename the function string_concat to concat_elements_string to be consistent with other modules?

This would be an excellent thing to do - I'll try and get it done later today

@alamb alamb merged commit 949ad43 into apache:master May 26, 2022
@alamb
Copy link
Contributor

alamb commented May 26, 2022

Or maybe just concat_string would enough 🤔

@alamb
Copy link
Contributor

alamb commented May 26, 2022

Proposed change in #1754

@HaoYang670
Copy link
Contributor Author

Thank you for your review!

@HaoYang670 HaoYang670 deleted the rename_to_concat_elements branch May 26, 2022 23:05
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.

Rename the string kernel to concatenate_elements
4 participants