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

Remove normalize method from TemporalType #472

Merged
merged 3 commits into from
Aug 12, 2024

Conversation

hovaesco
Copy link
Member

@hovaesco hovaesco commented Jul 16, 2024

Description

Remove normalize method from TemporalType.
Related to #449

Non-technical explanation

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Jul 16, 2024
@hashhar
Copy link
Member

hashhar commented Aug 2, 2024

Resolves #449

I don't think this PR does it. It just removes dead code. We're missing tests for values which lie in DST gap or are doubled.

The existing tests for this which were added in the beginning only worked in the timezone of the author, not CI, in CI the session zone is such that those values don't create DST ambiguities.

Please adjust commit message to say something like This code has been dead/broken since 2b9ca0c2e59496e168907fec1df7a50f26976dce.

Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

LGTM % commit message

@hovaesco
Copy link
Member Author

hovaesco commented Aug 2, 2024

Adjusted the commit message.

@hashhar
Copy link
Member

hashhar commented Aug 2, 2024

please mind ci.

Due to the compability issue with krb5 newest release.
@hovaesco
Copy link
Member Author

hovaesco commented Aug 6, 2024

@hashhar please have a look at two last commits which fix CI.

@hashhar hashhar merged commit d4bf4b8 into trinodb:master Aug 12, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants