Skip to content

Conversation

@jdaugherty
Copy link
Contributor

This PR merges the mongodb & hibernate5 projects.

jamesfredley and others added 30 commits October 25, 2024 15:28
Use grails-bom, grails-bom h2 version, fix h2 tests & restore domain inheritance tests
fix(deps): update spring boot to v3.3.5
…bSpec, Remove @ignore on tests fixed by classpath scanning fixes
…-validate

fix apache#1155 - skipValidate logic in GormValidateable causes missed validations and is inconsistent
Update error exceptions from javax to jakarta
Rename hibernate5Version -> hibernateVersion
…-services-test

restore examples grails data services tests
jamesfredley and others added 4 commits March 11, 2025 12:29
- rename directory to include grails-data
- distinguish migraiton plugin project as hibernate5 (leave artifact as is for now)
- workaround for asset pipeline apache#177
@codeconsole
Copy link
Contributor

codeconsole commented Mar 11, 2025

@jamesfredley so there is no ticket or discussion on why exactly they need to be built and released together? All I am wanting to understand is what is the gain in merging these 2 repos into data-mapping and how that can't be accomplished in another way while keeping them apart. I think this merge will be a major mistake.

@jdaugherty
Copy link
Contributor Author

@codeconsole we have talked about this for months at this point. Every time you've been the only person to disagree and everyone else , especially those that perform the builds, have said this is for the best.

We have agreed to it multiple times previously, and then backed off because it wasn't technically possible. With the project restructures, we realized it was possible given the previous approval, we are moving forward.

For the short term future, this project will be maintained. We can't easily do that when it's separate - especially given the limited volunteer time. We are about to repackage as part of the move. This is being moved and built to be continued supported until we have a hibernate 6 implementation.

@codeconsole
Copy link
Contributor

@jdaugherty yes, I was the 1 person to disagree with 3 other people during a 3 minute discussion because I think it is an incorrect approach to solve an underlying problem that still has yet to be addressed: the unnecessary coupling of dependencies and the bom. This all could have been cleaned up and resolved in a week. I have been strongly against these consolidations and that is main reason why I haven't been contributing.

Rather than focus on compatibility and finishing the Spring Boot 2 > 3 upgrade, we have lost 3 months doing project consolidations without even being able to keep up to date with patch/bug releases of the underlying Spring Boot/Spring Security/Spring Framework. M2 was released with outdated patch versioning that came out 2 weeks prior.

I still don't understand how merging in 3,299 unrelated commits and 91 open issues from a project based on dependencies that haven't been updated in 2 years helps anything. I think decisions as significant as this should warrant more time than 3 minutes discussing their rationality and purpose. At the very least they should have a clear definition on what it is exactly they are accomplishing other than eliminating a build file.

Is it really that difficult to release gorm hibernate 5 independently? What exactly is this accomplishing?

@jdaugherty
Copy link
Contributor Author

The meeting minutes show this was discussed for multiple weeks with multiple people. Please do not represent this as an immediate decision or one off. You were the only person who disagreed. Every time we discussed it, it was approved.

I get that you do not use hibernate 5, but many people do. The project consolidation occurred to facilitate development. Having separate projects has caused a lot of pain. We have enumerated that pain and even offered to show you it. You have ignored any pleas for help with the build.

All of these merges have not prevented contributions. We welcome them. Your further help on Spring upgrades or the multiple layout issues due to the sitemesh upgrade is welcome.

Almost every project merger has revealed major issues. These mergers have had a huge benefit as a result. We are volunteers and we are positioning ourselves for faster releases. Merging the project does not affect any function of grails; it only helps us release faster.

@codeconsole
Copy link
Contributor

We have enumerated that pain and even offered to show you it. You have ignored any pleas for help with the build.

That is not a true statement . Who, and when, ever offered to show me anything of the such? In fact, the opposite. Not once did anyone reach out to me to help with the build. I actually offered to fix all the build/release issues, but was voted against in favor of consolidations of which, I repeat, still haven't even addressed the underlying dependency coupling issue.

@jdaugherty So how much time would you say this was discussed in the weekly meeting? Maximum of what, collectively 5 minutes then? During that time was there ever a thorough explanation detailing why it was needed? no. Is there a solid reference to any documentation anywhere on why it is needed? no. Wouldn't an issue or a GitHub discussion be a better place for such a discussion to be done?

I get that you do not use hibernate 5, but many people do.

That is also not true. I do use hibernate in a few projects and in the other I use mongo and that has zero impact on my opinion.

I am still looking for a basic explanation on how exactly merging hibernate 5 into grails-data-mapping speeds up anything.
I think it is going to slow things down and make the overall project more difficult to maintain. If I didn't strongly feel that way, I wouldn't even be wasting my time having this discussion.

How exactly is this going to speed up the release process? What is specifically causing an issue by keeping them separate?

Copy link
Contributor

@matrei matrei 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 not sure the TCK tests are run at the moment, and I don't think the documentation output is what it needs to be (after fixing some issues so it runs).

* rename projects to have common prefixes
* spell mistakes
* build action cleanup
* update documentation
* fix styling
* serialize mongo tests because they have a shared resource (the db)
* update asset serialization (asset-pipeline#177) to be lazy
* split the simple map datastore into it's own library
* split the tck up into domains / base spec / tests
* ensure the tck only runs on the simple, hibernate5, and mongo implementations
@jdaugherty
Copy link
Contributor Author

jdaugherty commented Mar 13, 2025

@jamesfredley and I found that the tck tests weren't always running. At least for the gorm-test / simple datastore tests, they're not running in 8.x. You can confirm this by Changing the SimpleMapTestSuite and setting the test to BuiltinUniqueConstraintWorksWithTargetProxiesConstraintsSpec.

They all run in hibernate5 / mongodb / simple datastore. Some of the hibernate/mongo tests had to be updated to work so I suspect they weren't all running.

While there aren't failures in mongo/hibernate, there are failures in the simple datastore. I've left them un-ignored for now - it's 4 test classes. We should talk about how to handle those scenarios.

As part of "fixing" the intellij bug that was the original blocker, I've split the tck up into 3 packages. This lets intellij behave - it was locking up in the Grails plugin when the tck project was inspected. I suspect it's because we were replacing class files and there were duplicates in various modules.

I've also split up the simple data store away from the tests, and the test library no longer is published. We can decide if we want to publish our tests, but I don't think any of our projects do that. The tck is obviously special and is still published.

@jdaugherty jdaugherty requested a review from matrei March 13, 2025 03:11
@jdaugherty
Copy link
Contributor Author

jdaugherty commented Mar 13, 2025

we added pending feature for the failing tests in 8.x to make a clean test run

Copy link
Contributor

@matrei matrei left a comment

Choose a reason for hiding this comment

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

This was a beast! Good work!

@jamesfredley jamesfredley merged commit fcfec6c into apache:9.0.x Mar 13, 2025
22 checks passed
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.

10 participants