-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Fix split packages issue in Hibernate ORM with Panache #18743
Conversation
The list is incomplete for now but we will add the others after fixing each issue.
For now, only deprecate the old annotation, it will be removed in 2.2.
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.
Hibernate with Panache part is OK, I suggest a small change but it's not important.
if (parameter.isAnnotationPresent(ProjectedFieldName.class)) { | ||
final String name = parameter.getAnnotation(ProjectedFieldName.class).value(); | ||
if (name.isEmpty()) | ||
if (name.isEmpty()) { | ||
throw new PanacheQueryException("The annotation ProjectedFieldName must have a non-empty value."); | ||
} | ||
parameterName = name; | ||
} else if (parameter.isAnnotationPresent(io.quarkus.hibernate.orm.panache.ProjectedFieldName.class)) { | ||
final String name = parameter.getAnnotation(io.quarkus.hibernate.orm.panache.ProjectedFieldName.class).value(); | ||
if (name.isEmpty()) { | ||
throw new PanacheQueryException("The annotation ProjectedFieldName must have a non-empty value."); | ||
} |
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.
You could have squashed these two if on the same condition using an ||, this would avoid the small code duplication.
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.
Well not really as I get the value from a different annotation. I think it's good enough as it is given we will drop one of the branch in 2.2.
No description provided.