-
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 hbm.xml builds with CompositeAttributes #22701
Conversation
digsb
commented
Jan 6, 2022
- There was a StackOverflowError when project with hibernate-orm hbm xml contains a Composite Attribute.
- Fixes Issue: java.lang.StackOverflowError during build when using hibernate-orm with CompositeAttribute within hbm.xml #22685
This comment has been minimized.
This comment has been minimized.
Can you squash the commits in your PR? |
@gastaldi Yes will squash commits. |
@gastaldi Done |
@yrodiere could you please review this one? Thanks! |
@gsmet he already did in #22685 (comment), that's why I approved 😉 |
Yes, I just want him to also have a look at the tests. |
This comment has been minimized.
This comment has been minimized.
@gastaldi - There is a CI Build that has failed. I am not sure if this is related to my change, can you guide me on how to resolve this? |
@digsb try rebasing your branch with the latest main. Also as a good practice always create a separate branch when creating PRs (don't use |
No really, it was not related, there was no need to rebase. I see a merge commit now, I will try to do a proper rebase. |
I force pushed a rebase. |
…onentTest) "Complex" could mean anything; at least "Component" tells us we're testing the "component" feature of hbm.xml. Also, yes, I agree "simple" in "hbm-simple.xml" is not great either. I just used this to mean "there's nothing special about it, we simply want to test the basics of HBM file support".
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.
LGTM! Thanks a lot @digsb .
I pushed a few trivial changes to improve the tests; nothing critical.
Let's merge once CI is green.
Nice team work, thanks everyone! |
@digsb FYI, I plan to backport this to 2.6.2.Final that I will release on Monday. |