Rename internal methods #10204
Rename internal methods #10204
Conversation
| /** @var array<string,int> */ | ||
| // @phpcs:ignore | ||
| /** @var array<class-string<MappingAttribute>, int> */ | ||
| protected $entityAttributeClasses = [ |
There was a problem hiding this comment.
Let me know if you think this should be a private property, I don't know why $entityAnnotationClasses is exposed or why the class isn't final.
There was a problem hiding this comment.
Probably from the dark ages of PHP.
There was a problem hiding this comment.
$entityAnnotationClasses is an extension point of the AnnotationDriver from Persistence 2. However, we already override all functionality that uses the property.
Theoretically, you could invent your own attributes and extend AttributeDriver to make use of them. Either way, a protected property is a bad extension point. The property is used in the isTransient() method only.
My suggestion: Move the array into a private constant, deprecate the property and tell people to override isTransient() instead of overriding or changing the property. For the time being, our implementation of isTransient() has to use the property, but we can switch it to the new constant on 3.0.x.
| /** @var array<string,int> */ | ||
| // @phpcs:ignore | ||
| /** @var array<class-string<MappingAttribute>, int> */ | ||
| protected $entityAttributeClasses = [ |
There was a problem hiding this comment.
Probably from the dark ages of PHP.
|
I'm not sure though if this does break something when someone extended it. |
| /** @var array<string,int> */ | ||
| // @phpcs:ignore | ||
| /** @var array<class-string<MappingAttribute>, int> */ | ||
| protected $entityAttributeClasses = [ |
There was a problem hiding this comment.
$entityAnnotationClasses is an extension point of the AnnotationDriver from Persistence 2. However, we already override all functionality that uses the property.
Theoretically, you could invent your own attributes and extend AttributeDriver to make use of them. Either way, a protected property is a bad extension point. The property is used in the isTransient() method only.
My suggestion: Move the array into a private constant, deprecate the property and tell people to override isTransient() instead of overriding or changing the property. For the time being, our implementation of isTransient() has to use the property, but we can switch it to the new constant on 3.0.x.
greg0ire
force-pushed
the
rename-internal-methods
branch
3 times, most recently
from
ce40c10
to
56ef43a
Compare
These are internal, so it should be fine.
Cannot tell what this ignores.
greg0ire
force-pushed
the
rename-internal-methods
branch
from
ea0e987
to
12f0674
Compare
No description provided.