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

feat(cpp-client): Honor Arrow TimeUnit when converting Timestamp and Time64 to ColumnSource #6609

Merged
merged 1 commit into from
Feb 8, 2025

Conversation

kosak
Copy link
Contributor

@kosak kosak commented Feb 2, 2025

This PR has adds two tests but leaves one disabled. The disabled one will fail on the main branch; that is why it is disabled.

To run all the tests, execute this command.

./gradlew :cpp-client:testCppClient

Then, if you rebase this branch onto your nate/nightly/barrage_types branch, you can re-enable the disabled test by editing the file cpp-client/deephaven/tests/src/time_unit_test.cc and removing the tag [.hidden]. Currently that tag is at line 75.

You should then be able to run all the unit tests with the same gradlew command above and they should all pass.

@kosak kosak added NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed. labels Feb 2, 2025
@kosak kosak self-assigned this Feb 2, 2025
@kosak kosak requested a review from nbauernfeind February 2, 2025 03:33
Copy link
Member

@nbauernfeind nbauernfeind left a comment

Choose a reason for hiding this comment

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

LGTM; the R tests pass. I believe I've also been able to run with the 2nd cpp test going, but not 100% certain (reaching out over slack to see if I re-enabled it properly.).

@kosak kosak merged commit 6252118 into deephaven:main Feb 8, 2025
18 checks passed
@kosak kosak deleted the kosak_time-units branch February 8, 2025 01:21
@github-actions github-actions bot locked and limited conversation to collaborators Feb 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants