Skip to content

replace @XxxxQueryOptions with one @StaticQueryOptions annotation#984

Merged
njr-11 merged 2 commits intojakartaee:mainfrom
gavinking:staticqueryoptions
Mar 24, 2026
Merged

replace @XxxxQueryOptions with one @StaticQueryOptions annotation#984
njr-11 merged 2 commits intojakartaee:mainfrom
gavinking:staticqueryoptions

Conversation

@gavinking
Copy link
Copy Markdown
Member

@gavinking gavinking commented Mar 21, 2026

alternate solution to #806, since @njr-11 objected to #933.

@gavinking gavinking added the candidate-for-4 Good candidate for JPA 4 label Mar 21, 2026
@gavinking gavinking requested a review from njr-11 March 21, 2026 18:15
@gavinking gavinking mentioned this pull request Mar 21, 2026
Draft
@gavinking gavinking changed the title replace @XxxxQueryOptions with one @StaticQuryOptions annotation replace @XxxxQueryOptions with one @StaticQueryOptions annotation Mar 21, 2026
@gavinking gavinking force-pushed the staticqueryoptions branch from b06c703 to 96ef425 Compare March 21, 2026 18:19
@gavinking gavinking marked this pull request as ready for review March 21, 2026 18:19
@gavinking
Copy link
Copy Markdown
Member Author

gavinking commented Mar 23, 2026

@njr-11 @sebersole I would like to hear your feedback on this.

njr-11
njr-11 reviewed Mar 23, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@njr-11 njr-11 left a comment

Choose a reason for hiding this comment

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

I'm fine with the single @StaticQueryOptions annotation that covers both read and write options. I added one comment where one of the Jakarta Data usages for WriteQueryOptions was left out when combining

@gavinking @njr-11
Apply suggestion from @njr-11
8915873 
Co-authored-by: Nathan Rauh <nathan.rauh@us.ibm.com>
@gavinking
Copy link
Copy Markdown
Member Author

gavinking commented Mar 24, 2026

Thanks, Nathan.

@njr-11 njr-11 merged commit cf47638 into jakartaee:main Mar 24, 2026
3 checks passed
@njr-11
Copy link
Copy Markdown
Member

njr-11 commented Mar 24, 2026

Oops - sorry about the accidental merge. I thought I had a completely different PR up, not this one.

@gavinking
Copy link
Copy Markdown
Member Author

gavinking commented Mar 24, 2026

No worries. I'm still waiting to hear what @sebersole thinks, but if he doesn't object then we can just leave it merged.

@sebersole
Copy link
Copy Markdown
Contributor

sebersole commented Mar 25, 2026

Personally I'm not a fan of "one annotation(s) to rule them all" when you you have to explicitly say, "well, this option is only valid in one of the cases, and this one too, and this one blows up if you try to use it here...".

But I really have no objection nor honestly any skin in this particular game - Hibernate will support whatever y'all decide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@njr-11 njr-11 njr-11 approved these changes

Assignees

No one assigned

Labels

candidate-for-4 Good candidate for JPA 4 PRIORITY

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gavinking @njr-11 @sebersole