[2.1] Deprecate AbstractIdGenerator #2011
Conversation
| * @return mixed | ||
| */ | ||
| abstract public function generate(DocumentManager $dm, object $document); | ||
|
|
There was a problem hiding this comment.
I was thinking about adding a __construct emitting E_USER_DEPRECATED but chances are that if somebody has overridden the constructor, the parent's one might be not called
There was a problem hiding this comment.
Since we don't have a constructor to hook into, you may also trigger the deprecation on top of the file. It will alert people that they're using deprecated functionality, even though it won't point out every single occurrence where this is used.
There was a problem hiding this comment.
Not sure if it's a problem, but since our generators are extending the abstract they'll also generate a deprecation notice. And we can't remove it from the inheritance chain as that'd be a BC break too :/
There was a problem hiding this comment.
Ah, yeah. Difficult one, that 🤔
malarzm
force-pushed
the
id-generator-interface
branch
from
c064cba to
8c58818
Compare
|
One minor thing: this needs to be mentioned in the upgrade notes. |
alcaeus
changed the title
|
Pushed back to 2.1 to not deprecate stuff in a 2.0 release 😉 |
| /** | ||
| * AbstractIdGenerator | ||
| * @deprecated AbstractIdGenerator was deprecated in 2.0 and will be removed in 3.0. Implement IdGenerator interface instead. |
There was a problem hiding this comment.
Needs to be updated for 2.1:
| * @deprecated AbstractIdGenerator was deprecated in 2.0 and will be removed in 3.0. Implement IdGenerator interface instead. | |
| * @deprecated AbstractIdGenerator was deprecated in doctrine/mongodb-odm 2.1 and will be removed in 3.0. Implement IdGenerator interface instead. |
| @@ -1370,7 +1370,7 @@ public function isCollectionValuedEmbed(string $fieldName) : bool | |||
| /** | |||
| * Sets the ID generator used to generate IDs for instances of this class. | |||
| */ | |||
| public function setIdGenerator(AbstractIdGenerator $generator) : void | |||
| public function setIdGenerator(IdGenerator $generator) : void | |||
There was a problem hiding this comment.
This is a BC break since ClassMetadata is not final - not sure how to best solve this at this time :|
There was a problem hiding this comment.
To add a quick note, I think the impact of the BC break is small enough to risk doing this. I would also make the class @final to indicate that people really shouldn't be messing with it. We can then consider making it final in 3.0.
malarzm
force-pushed
the
id-generator-interface
branch
from
8c58818 to
282b399
Compare
There's no reason to provide an abstract class instead of an interface. Since this is a BC break I propose we do this for 2.0 although I'm eager to deprecate the class in 1.3 and remove the abstract in 2.0 already :)