Skip to content

Conversation

jrenaat
Copy link
Member

@jrenaat jrenaat commented Jul 30, 2025


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


https://hibernate.atlassian.net/browse/HHH-19015

@jrenaat jrenaat requested a review from gavinking July 30, 2025 14:17
@jrenaat jrenaat force-pushed the HHH-19015_findoptions branch from 380f714 to 855d16a Compare July 30, 2025 14:21
Copy link
Member

@sebersole sebersole left a comment

Choose a reason for hiding this comment

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

I think adding the deprecations to 7.1 is fine.
Made suggestions to apply that.

@jrenaat jrenaat force-pushed the HHH-19015_findoptions branch from 855d16a to 9605db7 Compare July 30, 2025 14:39
@jrenaat jrenaat force-pushed the HHH-19015_findoptions branch from 9605db7 to 7623f30 Compare July 30, 2025 14:46
@sebersole sebersole merged commit 535be5f into hibernate:main Jul 30, 2025
26 checks passed
@gavinking
Copy link
Member

Does it make sense to deprecate the method without also deprecating IdentifierLoadAccess?

@sebersole
Copy link
Member

Well, at least initially it makes the most sense (simplicity) to simply keep using that infrastructure internally. So for now, I'd say no. After I get the #find overloads correct we could look into converting IdentifierLoadAccess uses to just use FindOptions directly. But for now, I say no

@jrenaat jrenaat deleted the HHH-19015_findoptions branch July 31, 2025 14:37
@sebersole
Copy link
Member

Well, actually... the interface, yeah I think that does make sense

@sebersole
Copy link
Member

@DavideD According to the note on IdentifierLoadAccessImpl, Reactive extends all of this so this deprecation will affect you I think.

@DavideD
Copy link
Member

DavideD commented Jul 31, 2025

Thanks, I've seen it. I don't remember exactly how we use it, but it should be fine

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants