Support other subclasses of EntityManagerInterface too
#180
Conversation
Also require PHP 7.4+ and expand use of type constraints
| * @param Entity $entity | ||
| * | ||
| * @return UuidInterface | ||
| * | ||
| * @throws Exception | ||
| */ | ||
| public function generate(EntityManager $em, $entity) | ||
| public function generateId(EntityManagerInterface $em, $entity): UuidInterface |
There was a problem hiding this comment.
This introduces a BC break, right? If anyone extends UuidGenerator and overrides this method, making this change will result in an error like:
Fatal error: Declaration of Foo::generate(EntityManager $em, $entity) must be compatible with Ramsey\Uuid\Doctrine\UuidGenerator::generate(EntityManagerInterface $em, $entity): UuidInterface
There was a problem hiding this comment.
@ramsey Yes, sorry I wasn't clear. This is what I was trying to get at with:
I suggest this targets a new major release.
I would also expect the bump in minimum PHP version to necessitate a new major version as that's also a BC break, hence my thinking that the two changes could go hand in hand.
There was a problem hiding this comment.
A bump in minimum PHP version shouldn't be considered a BC break IMHO. The reasoning is explained under "Why dropping PHP support in a minor version is not a BC break" at https://www.doctrine-project.org/2017/07/25/php-7.1-requirement-and-composer.html
Also require PHP 7.4+ and expand use of type constraints
|
Thank you for contributing! 🎉 I committed this in 53955b7 |
Changes from ramsey/uuid-doctrine#180 are now released as per ramsey/uuid-doctrine#179
Description
Support any
EntityManagerInterfacein newgenerateId()methods, and also in existinggenerate()s.Also require PHP 7.4+ and expand use of type constraints.
I suggest this targets a new major release.
Motivation and context
Detailed in thebiggive/matchbot#338
How has this been tested?
Unit tests run and added to. Fork used in thebiggive/matchbot#338 both with real local calls and unit tests.
Types of changes
I believe this is backwards incompatible only insofar as PHP version support, so I have not made any documentation changes or identified any that are needed.
PR checklist