[DoctrineBridge] Remove dead code in DoctrineType #20312
Conversation
7db5058 to
9b49723
Compare
| @@ -108,15 +99,9 @@ public function getQueryBuilderPartsForCachingHash($queryBuilder) | ||
| return false; | ||
| } | ||
|
|
||
| public function __construct(ManagerRegistry $registry, PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null) | ||
| public function __construct(ManagerRegistry $registry) |
There was a problem hiding this comment.
I don't think we should remove this since it would be a BC break for anyone using the Form component and the DoctrineBridge without the full stack.
There was a problem hiding this comment.
A BC break for people doing weird things too as says @stof :D
There was a problem hiding this comment.
Anyway this is an abstract class and we can't do that change.
There was a problem hiding this comment.
This is not a BC break because PHP does not care about additional arguments that you pass to constructor/methods. Checks this http://symfony.com/doc/current/contributing/code/bc.html#id13
There was a problem hiding this comment.
Given a class that extends of DoctrineType (currently) if the __constructor() was overwritten would look like this:
public function __construct(ManagerRegistry $registry, PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null, $extraData = null)
{
parent::__constructor($registry, $propertyAccessor, $choiceListFactory);
$this->extraData = $extraData;
}It doesn't break after this PR.
There was a problem hiding this comment.
Ok after all it's private method
|
Thanks @yceruto, good catch! Status: Reviewed |
|
Thank you @yceruto. |
This PR was merged into the 3.1 branch. Discussion ---------- [DoctrineBridge] Remove dead code in DoctrineType | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | no | New feature? | no | BC breaks? | no (Only the last arguments of a method may be removed [[3]](http://symfony.com/doc/current/contributing/code/bc.html#id13) ) | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - After optimization of DoctrineChoiceLoader (86b2ff1 in #18359) this code died :) Commits ------- 9b49723 remove dead code
…n (yceruto) This PR was submitted for the master branch but it was merged into the 3.2 branch instead (closes #21493). Discussion ---------- [DoctrineBridge] Remove dead code in DoctrineOrmExtension | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT After optimization in #20312 and #18359 Commits ------- 0cd8bf8 Remove dead code
After optimization of DoctrineChoiceLoader (86b2ff1 in #18359) this code died :)