-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[SoftDeletable] Fix ArrayAccess deprecation on FieldMapping #2856
base: main
Are you sure you want to change the base?
[SoftDeletable] Fix ArrayAccess deprecation on FieldMapping #2856
Conversation
I was worried about the compatibility to doctrine/orm ^2.14.0 |
Very nice |
You don't need Composer's API here to do a version check when you're already making a "is the meta an instance of this object" check as well. The instance check is more important than the version check because both the ORM and MongoDB ODM are supported. And while this will fix this one instance of the deprecation, there are literally dozens of cases that need to be fixed if the intent is to silence all deprecations. |
Ok, it seems fine without checking the version.
I think to keep consistency in the codebase, it is better to align every part where FieldMapping is used, instead of a mixed situation. |
Gedmo\SoftDeleteable\Mapping\Event\Adapter\ORM::getDateValue
Because those private methods originally had the
I can't argue that, but as I've mentioned elsewhere, there are dozens of locations where the mapping data is used and needs to account for the MongoDB ODM structure (arrays), the ORM 2.x structure (arrays), and the ORM 3.x structure (DTOs with a deprecated ArrayAccess implementation for B/C). Just fixing these one at a time because someone noticed messages in their deprecation logs is going to end up in more inconsistency until they're all fixed. I'm not in a spot to stop someone from making these patches as they need to land eventually, but IMO fixing these deprecations (which is just adding extra noise to logs, not breaking anything) is a rather low priority compared to some of the other stuff on the issues list. |
[Unreleased]
Fixed