-
Notifications
You must be signed in to change notification settings - Fork 8
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
Update JSON type mapping with io.hypersistence:hypersistence-utils-hibernate
#7
base: main
Are you sure you want to change the base?
Conversation
d83839f
to
fd78548
Compare
…ypersistence:hypersistence-utils-hibernate'
fd78548
to
6724d68
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! This already looks quite good. I have not yet reviewed the recipe in detail, but I've added a few comments to get you started.
src/main/java/org/openrewrite/hibernate/hibernate60/MigrateHypersistenceUtils61Types.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/hibernate/hibernate60/MigrateHypersistenceUtils61Types.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/hibernate/hibernate60/MigrateHypersistenceUtils61Types.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/hibernate/MigrateToHibernate61Test.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/hibernate/MigrateToHibernate61Test.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/hibernate/hibernate60/MigrateHypersistenceUtils61Types.java
Outdated
Show resolved
Hide resolved
return Preconditions.check(Preconditions.and( | ||
new FindImports("org.hibernate.annotations.Type").getVisitor(), | ||
Preconditions.or( | ||
new FindImports("com.vladmihalcea..*").getVisitor(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious as to the inclusion of com.vladmihalcea
here, as it's the only reference to the package in this recipe/visitor. Since we include MigrateHypersistenceUtils61Types
in src/main/resources/META-INF/rewrite/hibernate-6.yml
after the ChangePackage
from com.vladmihalcea
to io.hypersistence.utils
it should not be necessary to check for the old package if people run MigrateToHypersistenceUtilsHibernate6.0
or MigrateToHibernate61
. Would you agree that we can then likely remove this check on com.vladmihalcea
and simplify the precondition?
Welcome back @iuliiasobolevska ! I've gone ahead an added a few additional commits to this one taking in some of the feedback above, as well as the parser classpath entry added in #8. There's now a small difference between the expected and actual output, using |
io.hypersistence:hypersistence-utils-hibernate
src/main/java/org/openrewrite/hibernate/hibernate60/MigrateHypersistenceUtils61Types.java
Outdated
Show resolved
Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…/issue-3-recipe-for-migrating-hypersistence-utils-types
What's changed?
@TypeDef
and@TypeDefs
annotations have been removed, source.JSON type mapping changed in Hibernate 6 and hypersistence-utils 6, source.
What's your motivation?
Anything, in particular, you'd like reviewers to focus on?
Any OpenRewrite tips and tricks to make it more concise and beautify the code are highly appreciated.
Any additional context
Taking a cautious approach and only removing
@TypeDef
and@TypeDefs
annotations when used in combination withhypersistence-utils
.This recipe doesn't cover list-alike type mapping.
Checklist
./gradlew licenseFormat