-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Remove ability to clear the UoW partially #9471
Conversation
*/ | ||
public function clear($entityName = null): void | ||
public function clear($objectName = null): void |
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.
Note: We have to keep the parameter because the parent interface ObjectManager
demands it. Because we don't use it anymore, It decided to rename it as declared by the parent interface.
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.
Looks like it is not deprecated in this other implementation: https://github.com/doctrine/mongodb-odm/blob/2.3.x/lib/Doctrine/ODM/MongoDB/DocumentManager.php#L696-L707
Not sure what is the right move for doctrine/persistence
… dropping the parameter from the interface would work, but would make static analysis frown:
Semantically, it feels like there should be an extended interface: https://phpstan.org/r/57d5847a-1f74-4598-8fe4-7ca319263d10
Aaaand I just noticed that we already picked a way: doctrine/persistence#174
Maybe we should talk about the next major releases of persistence and ORM, and which one should happen before the other one? (I'd say persistence)
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.
Yes, we should have this discussion. However, as long as we support Persistence 2, this change in its current form is correct. As soon as we drop Persistence 2 (e.g. by moving to Persitence 3) we can revisit this method and drop the parameter.
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.
Yes, should have said this wasn't a blocker, sorry.
*/ | ||
public function clear($entityName = null): void | ||
public function clear($objectName = null): void |
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.
Looks like it is not deprecated in this other implementation: https://github.com/doctrine/mongodb-odm/blob/2.3.x/lib/Doctrine/ODM/MongoDB/DocumentManager.php#L696-L707
Not sure what is the right move for doctrine/persistence
… dropping the parameter from the interface would work, but would make static analysis frown:
Semantically, it feels like there should be an extended interface: https://phpstan.org/r/57d5847a-1f74-4598-8fe4-7ca319263d10
Aaaand I just noticed that we already picked a way: doctrine/persistence#174
Maybe we should talk about the next major releases of persistence and ORM, and which one should happen before the other one? (I'd say persistence)
fdf6b4a
to
2d32723
Compare
This reverts commit 1c67f42.
Clearing the UoW partially has been deprecated for quite a while. Let's remove the feature from the 3.0.x branch.