Skip to content

GH-33143: [C++] Naming and doc/test changes for local_time compute kernel #34263

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
Merged
rok merged 2 commits into from
Feb 23, 2023
Merged

GH-33143: [C++] Naming and doc/test changes for local_time compute kernel #34263

rok merged 2 commits into from
Feb 23, 2023

Conversation

@rok
Copy link
Member

@rok rok commented Feb 20, 2023 •
edited by github-actions bot
Loading

Rationale for this change

A better naming for local_time kernel was proposed in post merge review of #34208.

What changes are included in this PR?

Change local_time to local_timestamp and related docs/test changes.

Are these changes tested?

Yes.

Are there any user-facing changes?

Changing local_time to local_timestamp is a user facing change. But since it was not yet released we can probably treat it as non-breaking.

@rok rok mentioned this pull request Feb 20, 2023
Merged
@rok rok force-pushed the 33143-get-walltime-follow-up branch from 43b4d34 to 76c8acd Compare February 20, 2023 16:46
@kou
Copy link
Member

kou commented Feb 20, 2023

Could you open a new issue for this?
This is not a MINOR change.
See also: https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#minor-fixes

@jorisvandenbossche jorisvandenbossche changed the title MINOR: [C++] Naming and doc/test changes for local_time compute kernel GH-33143: [C++] Naming and doc/test changes for local_time compute kernel Feb 20, 2023
@github-actions
Copy link

github-actions bot commented Feb 20, 2023

@rok
Copy link
Member Author

rok commented Feb 21, 2023

Sorry @kou this is indeed not minor. @jorisvandenbossche thanks for linking to the original issue, I think this mapping is satisfactory?
I think this is also ready for review.

jorisvandenbossche
jorisvandenbossche reviewed Feb 21, 2023
View reviewed changes
@westonpace
Copy link
Member

westonpace commented Feb 22, 2023

This looks fine to me.

@rok rok merged commit 863cdd4 into apache:main Feb 23, 2023
@rok rok deleted the 33143-get-walltime-follow-up branch February 23, 2023 12:32
@ursabot
Copy link

ursabot commented Feb 23, 2023

Benchmark runs are scheduled for baseline = e0e740b and contender = 863cdd4. 863cdd4 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
[Failed ⬇️0.55% ⬆️0.0%] test-mac-arm
[Finished ⬇️1.79% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.48% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 863cdd42 ec2-t3-xlarge-us-east-2
[Failed] 863cdd42 test-mac-arm
[Finished] 863cdd42 ursa-i9-9960x
[Finished] 863cdd42 ursa-thinkcentre-m75q
[Finished] e0e740bd ec2-t3-xlarge-us-east-2
[Finished] e0e740bd test-mac-arm
[Finished] e0e740bd ursa-i9-9960x
[Finished] e0e740bd ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. 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

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

Reviewers

@jorisvandenbossche jorisvandenbossche jorisvandenbossche approved these changes

@AlenkaF AlenkaF Awaiting requested review from AlenkaF AlenkaF is a code owner

@westonpace westonpace Awaiting requested review from westonpace

Assignees

No one assigned

Labels

Component: C++ Component: Documentation Component: Python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[C++] Kernel to convert timestamp with timezone to wall time

5 participants

@rok @kou @westonpace @ursabot @jorisvandenbossche