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.

Sign up for GitHub

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

Jump to bottom

ARROW-15782: [C++] Fix Findre2Alt.cmake to check RE2_ROOT variable first #12513

Closed
wants to merge 1 commit into from
Closed

ARROW-15782: [C++] Fix Findre2Alt.cmake to check RE2_ROOT variable first #12513

wants to merge 1 commit into from

Conversation

sfc-gh-hyu
Copy link
Contributor

No description provided.

@github-actions
Copy link

https://issues.apache.org/jira/browse/ARROW-15782

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@lidavidm lidavidm changed the title ARROW-15782 [C++] Fix Findre2Alt.cmake to check RE2_ROOT variable first ARROW-15782: [C++] Fix Findre2Alt.cmake to check RE2_ROOT variable first Feb 25, 2022
@pitrou pitrou requested a review from kou March 1, 2022 15:13
kou
kou approved these changes Mar 1, 2022
View reviewed changes
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@kou kou closed this in 2039eda Mar 1, 2022
@ursabot
Copy link

ursabot commented Mar 1, 2022 •
edited
Loading

Benchmark runs are scheduled for baseline = 13045f4 and contender = 2039eda. 2039eda is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.46% ⬆️0.04%] test-mac-arm
[Finished ⬇️0.0% ⬆️3.21%] ursa-i9-9960x
[Finished ⬇️0.26% ⬆️0.04%] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@sfc-gh-hyu
Copy link
Contributor Author

@kou May I ask why this PR is closed? Looks like you approved the PR, but did not merge it.

@kou
Copy link
Member

kou commented Mar 1, 2022

GitHub says "closed" but your change was merged into master.
This is caused because we use our merge tool instead of GitHub's merge button.

See also: #12004 (comment)

It may be better that we document about this somewhere... Do you want to work on the documentation task?

@kou
Copy link
Member

kou commented Mar 1, 2022

Or we may be able to use GitHub's merge API instead of git push. Then GitHub will say "merged" not "closed".

@sfc-gh-hyu
Copy link
Contributor Author

Ah I see. Thanks! This indeed confuses me. Anyway, I'm glad this is accepted!

kszucs pushed a commit to kszucs/arrow that referenced this pull request Mar 3, 2022
@sfc-gh-hyu @kszucs
ARROW-15782: [C++] Fix Findre2Alt.cmake to check RE2_ROOT variable first
b92ef3b 
Closes apache#12513 from sfc-gh-hyu/hyu-fix-findre2

Authored-by: Haowei Yu <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
@asfimport asfimport mentioned this pull request Mar 3, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kou kou kou approved these changes

Assignees
No one assigned
Labels
Component: C++
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-hyu @ursabot @kou