Skip to content

GH-48592: [C++] Use starts_with/ends_with methods - #48614

Merged
pitrou merged 3 commits into
apache:mainfrom
jonahkel:string-starts-enhancement
Jan 5, 2026
Merged

GH-48592: [C++] Use starts_with/ends_with methods#48614
pitrou merged 3 commits into
apache:mainfrom
jonahkel:string-starts-enhancement

Conversation

@jonahkel

@jonahkel jonahkel commented Dec 21, 2025

Copy link
Copy Markdown
Contributor

Rationale for this change

The code previously used StartsWith and EndsWith utility functions. C++ now includes built-in starts_with and ends_with functions, rendering StartsWith and EndsWith unnecessary.

What changes are included in this PR?

All calls of StartsWith and EndsWith are replaces with starts_with and ends_with, respectively. The definition and tests for StartsWith and EndsWith have been deleted.

Are these changes tested?

Yes, all unit tests pass.

Are there any user-facing changes?

No.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@jonahkel

Copy link
Copy Markdown
Contributor Author

Ready for review.

@lidavidm

Copy link
Copy Markdown
Member

This PR appears to have pulled in several other commits - could you retry the rebase/merge?

@jonahkel

Copy link
Copy Markdown
Contributor Author

Sorry, I don't understand the issue. I believe the other commits appear because I fetched from the main repo and rebased my branch after I made the PR. Should I close and re-open the PR?

@lidavidm

Copy link
Copy Markdown
Member

A rebase shouldn't show the other commits in GitHub's UI, but this does. It seems the changes on main were rebased onto this branch, whereas the usual workflow is to rebase this branch onto main. (In particular, because this PR now "changes" all those files from those commits, a dozen committers have been pinged for "review".)

There's no need to re-make this PR, but could you share how the rebase was done?

@lidavidm

lidavidm commented Dec 22, 2025

Copy link
Copy Markdown
Member

For reference, what I usually do is git pull --rebase=interactive upstream main where upstream is git@github.com:apache/arrow. This requires force-pushing the branch afterwards. (If you prefer not to force-push the branch, then updating the branch via a merge commit is better - GitHub will understand that and won't treat the merge changes as "yours".)

@jonahkel

Copy link
Copy Markdown
Contributor Author

Ah, I understand. I indeed mistakenly ran git pull upstream main --rebase on my branch, instead of main on my fork.

@pitrou

pitrou commented Jan 5, 2026

Copy link
Copy Markdown
Member

@jonahkel Mmmh, can you perhaps try to re-generate this PR? There are so many non-relevant changes in the PR diff that it's not really possible to review it as is.

@jonahkel

jonahkel commented Jan 5, 2026

Copy link
Copy Markdown
Contributor Author

I apologize, I'll get it fixed.
Would you suggest reverting to before I pulled from the main repo and reimplementing changes?

@pitrou

pitrou commented Jan 5, 2026

Copy link
Copy Markdown
Member

Would you suggest reverting to before I pulled from the main repo and reimplementing changes?

If that helps, I would suggest starting from pristine git main and applying your changes as a patch.

@jonahkel

jonahkel commented Jan 5, 2026

Copy link
Copy Markdown
Contributor Author

Here's what I did:

  • Pulled the latest changes from upstream to the main branch of my fork.
  • Created a new feature branch
  • Cherry-picked my commits from my old feature branch
  • Reset the head of my old branch to point to the new one
  • Force pushed to my old feature branch

Can you let me know if this helped?

@pitrou

pitrou commented Jan 5, 2026

Copy link
Copy Markdown
Member

Can you let me know if this helped?

Well, definitely! The diff view is clean now.

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you @jonahkel !

@pitrou

pitrou commented Jan 5, 2026

Copy link
Copy Markdown
Member

@github-actions crossbow submit -g cpp

@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jan 5, 2026
@github-actions

github-actions Bot commented Jan 5, 2026

Copy link
Copy Markdown

Revision: 4a4b9b9

Submitted crossbow builds: ursacomputing/crossbow @ actions-b981cde2ee

Task Status
example-cpp-minimal-build-static GitHub Actions
example-cpp-minimal-build-static-system-dependency GitHub Actions
example-cpp-tutorial GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-cuda-cpp-ubuntu-24.04-cuda-13.0.2 GitHub Actions
test-debian-12-cpp-amd64 GitHub Actions
test-debian-12-cpp-i386 GitHub Actions
test-debian-experimental-cpp-gcc-15 GitHub Actions
test-fedora-42-cpp GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-bundled GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizer GitHub Actions

@pitrou

pitrou commented Jan 5, 2026

Copy link
Copy Markdown
Member

CI looks good apart from unrelated failure(s), will merge. Thank you for this @jonahkel !

@pitrou
pitrou merged commit 35717a7 into apache:main Jan 5, 2026
46 checks passed
@pitrou pitrou removed the awaiting committer review Awaiting committer review label Jan 5, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 35717a7.

There weren't enough matching historic benchmark results to make a call on whether there were regressions.

The full Conbench report has more details.

Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
### Rationale for this change

The code previously used StartsWith and EndsWith utility functions. C++ now includes built-in starts_with and ends_with functions, rendering StartsWith and EndsWith unnecessary.

### What changes are included in this PR?

All calls of StartsWith and EndsWith are replaces with starts_with and ends_with, respectively. The definition and tests for StartsWith and EndsWith have been deleted.

### Are these changes tested?

Yes, all unit tests pass.

### Are there any user-facing changes?

No.

* GitHub Issue: apache#48592

Authored-by: Jonah Kelman <jonahkel@umich.edu>
Signed-off-by: Antoine Pitrou <antoine@python.org>
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.

3 participants