Skip to content

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

Merged
rok merged 2 commits intoapache:mainfrom
rok:33143-get-walltime-follow-up
Feb 23, 2023
Merged

GH-33143: [C++] Naming and doc/test changes for local_time compute kernel#34263
rok merged 2 commits intoapache:mainfrom
rok:33143-get-walltime-follow-up

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