hibernate-orm
hibernate-orm copied to clipboard
add a method for use by quarkus extension
[Please describe here what your change is about]
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.
Thanks for your pull request!
This pull request does not follow the contribution rules. Could you have a look?
❌ All commit messages should start with a JIRA issue key matching pattern HHH-\d+
↳ Offending commits: [a7043419d4cc3407f0561b09313a1cf01ca65415, 6969e647775ef9d4555c2008a9b40d689fc6df20]
› This message was automatically generated.
@yrodiere @Sanne This is really, really fragile.
Would it be better to add a clone() method to MetadataImpl and have Quarkus call that instead?
Would it be better to add a
clone()method toMetadataImpland have Quarkus call that instead?
Looks like it should be called trim().
WDYT?
As discussed over Zulip, this is just one of many places where the integration between Quarkus and Hibernate ORM is very tight, to the point of being fragile, and that's the reason we only support running Quarkus with the exact version of Hibernate ORM it references in its BOM.
We can merge this if you want, but IMO it only makes sense if we also add a test to check that trim does what Quarkus expects (which is not immediately clear to me here, I'd have to dig into Quarkus code to remember -- might be worth a comment?).
Also FWIW MetadataImpl is internal, so unless you move trim to some SPI, there's a decent chance it'll just get removed in a later commit by someone not aware of what it's for.
Also FWIW
MetadataImplis internal, so unless you movetrimto some SPI, there's a decent chance it'll just get removed in a later commit by someone not aware of what it's for.
Which is why I added a comment.
The main issue is that the constructor that Quarkus was calling was not even commented as such.
Also FWIW
MetadataImplis internal, so unless you movetrimto some SPI, there's a decent chance it'll just get removed in a later commit by someone not aware of what it's for.Which is why I added a comment.
Fair enough.
LGTM, though I think changes that impact code we ship need a Jira issue, so we'll need to add that before merging.