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

Implement support for hibernate timestamp columns with timezone #728

Merged

Conversation

philipp-kleber-avelios
Copy link
Contributor

Since version 5 or 6, Hibernate started adding the with time zone suffix to columns of certain types.
This PR aims to correctly transfer the with time zone suffix from Hibernate to the corresponding Liquibase format with timezone.

@MalloD12
Copy link
Contributor

MalloD12 commented Dec 9, 2024

@philipp-kleber-avelios,

I think it would be nice to at least add some basic unit test for this. Would you mind doing it?

Thanks,
Daniel.

@philipp-kleber-avelios
Copy link
Contributor Author

@MalloD12,

Thank you for the feedback, I added a unit test and actually fixed a bug I noticed.
Let me know if this needs any further changes, thanks!

Copy link
Contributor

@MalloD12 MalloD12 left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you for adding a test for it and finding a bug.

A comment not related to this PR. I was thinking it would be nice to use Lombok in this extension. It would be useful to make a bit less verbose all the entity classes. I'll create a a separate ticket for it.

Again, thank you for your contributions.

Daniel.

@MalloD12
Copy link
Contributor

BTW, just to let you know, I created this enhancement #735 I mentioned I was going to create.

Thanks,
Daniel.

@filipelautert filipelautert force-pushed the timestamp-timezone-support branch from a34335b to c617c31 Compare December 26, 2024 12:53
@filipelautert filipelautert added this to the 1NEXT milestone Dec 26, 2024
@filipelautert filipelautert merged commit 3126d8c into liquibase:main Dec 26, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants