doctrine-extensions
ORM 3
I have to assume this PR is for an internal project or something, because there is no way a PR of this size is going to be reviewable.
@mbabker how then you think we should proceed? Most of the changes are required for ORM 3.
You're doing a lot more here than adding support for ORM 3:
- Changed the minimum PHP version
- Reformatted a LOT of code based on that change
- Removed ALL annotations support for both the MongoDB ODM and ORM
- Inlined an XML driver for the ORM for something?
(It's those middle two points that make the biggest impact on looking at this PR, FWIW, because of how many extra changes they are introducing without actually being a hard requirement)
https://github.com/doctrine-extensions/DoctrineExtensions/compare/main...mbabker:DoctrineExtensions:orm-3-merge has the minimum needed changes to make it work without changing dependency ranges (with that branch and https://github.com/doctrine-extensions/DoctrineExtensions/pull/2768 I have the tests passing locally), it just needs to be tested against both ORM 2.x and 3.x (as I've been working locally on that branch with only 3.0 installed) and the hard B/C issues addressed (like the union types that break PHP 7.4 compatibility; I know a lot of people's response at this point would be to say "well just drop support for it because it's EOL", but PHP 7.4 and 8.0 users still account for a not so insignificant number of recent installs and I personally would rather take the approach of seeing if it's practical to do something to not break that before creating a hard break, which PRs like https://github.com/doctrine-extensions/DoctrineExtensions/pull/2758 accomplish).
- ORM 3 needs PHP8.1
- Reformating is done by php-cs-fixer and rector, it could be removed a assume, but it's automated
- ORM 3 does not support annotations at all
- XML Driver is required as workaround for XSD validation
- YamlDriver is removed
- DoctrineExtension explicitly conflicts with ORM 3 by removing this conflict, we may end up with ORM 3 and older DoctrineExtension - thus broken code.
I see it as major release and ppl who are using 7.4 and 8.0 will stick with older version of ORM and DoctrineExtension.
I think our approaches to this are incompatible and that's causing me to look at this PR in a way that isn't going to work.
You're coming at this as though this PR will lead to a 4.0 release which immediately drops support for ORM 2.x and PHP versions older than 8.1, even though ORM 2.x is still going to be supported for a while. And as a result, you're making changes throughout the package that follow that assumption. Which means either this package stops supporting older ORM and PHP versions immediately, or there is an extra burden to bear by maintaining two branches (and thanks to all of the automated refactoring, there is an even bigger burden on upmerges if a fix is shipped on the older branch due to diverging code formatting rules).
The PRs that I've been iterating on work toward a forward compatibility layer without making a massive (and potentially disruptive) change to the package. The way I've been approaching it allows for the package to reach a point where both major versions could be supported with an overlap.
I just don't think that this approach is necessary to ship an ORM 3.0 compatible release. Aside from a handful of signature changes that require PHP 8 (namely the mixed type and union types), there isn't anything about the ORM 3.0 release that requires the scope of change included in this PR.
@mbabker I'm okay with closing my PR, please create your own PR and let's work together to make happen.
Build: https://github.com/oleg-andreyev/DoctrineExtensions/pull/1/checks
@oleg-andreyev Thank you for working on this :partying_face: It's actually last package that blocks upgrade to doctrine/orm 3 in our project
It's actually last package that blocks upgrade to doctrine/orm 3 in our project
+1 Is it project is... dead ? (last commit 3 month ago)
PS: Let's the forks begin ! :smile: (nobody need this)
Thanks @oleg-andreyev for the work you did here and sorry for taking so long, the main branch is now compatible with both ORM 2 and 3 (thanks to @mbabker mainly), it would be nice if people start testing it.
