Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[DoctrineBridge] Remove dead code in DoctrineType #20312

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 28, 2016
Merged

[DoctrineBridge] Remove dead code in DoctrineType #20312

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 1 addition & 16 deletions 17 src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,10 @@
use Symfony\Bridge\Doctrine\Form\EventListener\MergeDoctrineCollectionListener;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator;
use Symfony\Component\Form\ChoiceList\Factory\ChoiceListFactoryInterface;
use Symfony\Component\Form\ChoiceList\Factory\DefaultChoiceListFactory;
use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator;
use Symfony\Component\Form\Exception\RuntimeException;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\Options;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;

abstract class DoctrineType extends AbstractType
{
Expand All @@ -36,11 +32,6 @@ abstract class DoctrineType extends AbstractType
*/
protected $registry;

/**
* @var ChoiceListFactoryInterface
*/
private $choiceListFactory;

/**
* @var IdReader[]
*/
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A BC break for people doing weird things too as says @stof :D

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway this is an abstract class and we can't do that change.

Copy link
Member Author

@yceruto yceruto Oct 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@yceruto yceruto Oct 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok after all it's private method

{
$this->registry = $registry;
$this->choiceListFactory = $choiceListFactory ?: new CachingFactoryDecorator(
new PropertyAccessDecorator(
new DefaultChoiceListFactory(),
$propertyAccessor
)
);
}

public function buildForm(FormBuilderInterface $builder, array $options)
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.