Skip to content
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.

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

Fix deprecation on load fixtures command #280

Merged
merged 1 commit into from
May 29, 2019

Conversation

yceruto
Copy link
Contributor

@yceruto yceruto commented May 27, 2019

  1x: The "Doctrine\Bundle\FixturesBundle\Command\LoadDataFixturesDoctrineCommand" constructor expects a "Doctrine\Common\Persistence\ManagerRegistry" instance as first argument, not passing it will throw a \TypeError in DoctrineBundle 2.0.
    1x in Application::run from Symfony\Bundle\FrameworkBundle\Console
  1x: The "Doctrine\Bundle\DoctrineBundle\Command\DoctrineCommand::getContainer()" method is deprecated and will be removed in DoctrineBundle 2.0.
    1x in Application::run from Symfony\Bundle\FrameworkBundle\Console

@yceruto yceruto force-pushed the fix_deprecation branch from ade4fdb to e09a08a Compare May 27, 2019 21:58
Resources/config/services.xml Outdated Show resolved Hide resolved
@alcaeus alcaeus added this to the 3.2.0 milestone May 28, 2019
@yceruto yceruto force-pushed the fix_deprecation branch from e09a08a to 58f3892 Compare May 28, 2019 13:07
Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

One more suggestion, other than that, this looks good 👍

Command/LoadDataFixturesDoctrineCommand.php Show resolved Hide resolved
@yceruto yceruto force-pushed the fix_deprecation branch from 58f3892 to c966a81 Compare May 29, 2019 12:05
@yceruto yceruto force-pushed the fix_deprecation branch from c966a81 to 31d101b Compare May 29, 2019 12:20
@alcaeus
Copy link
Member

alcaeus commented May 29, 2019

Thanks @yceruto!

@alcaeus alcaeus merged commit ca5d3a0 into doctrine:master May 29, 2019
@yceruto yceruto deleted the fix_deprecation branch May 29, 2019 12:58
@yceruto
Copy link
Contributor Author

yceruto commented Jun 1, 2019

@alcaeus Is there a plan to release a new version to get rid of this warning everywhere? Thank you.

@alcaeus
Copy link
Member

alcaeus commented Jun 1, 2019

I’ll take time to do some housekeeping in the bundle next week 👍

@alcaeus
Copy link
Member

alcaeus commented Jun 7, 2019

3.2.0 is released. Thanks @yceruto!

alcaeus added a commit to alcaeus/DoctrineFixturesBundle that referenced this pull request Jun 12, 2019
With the changes made in doctrine#280, this bundle is no longer compatible with DoctrineBundle < 1.10. Thus, we need to raise the minimum version in a patch release.
alcaeus added a commit to alcaeus/DoctrineFixturesBundle that referenced this pull request Jun 12, 2019
With the changes made in doctrine#280, this bundle is no longer compatible with DoctrineBundle < 1.10. Thus, we need to raise the minimum version in a patch release.
), E_USER_DEPRECATED);
}

parent::__construct($doctrine);

Choose a reason for hiding this comment

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

Hello,

The Symfony Command constructor argument is waiting for a string and not an object. https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/Console/Command/Command.php#L69

On previous SF versions (without PHP 7 type), it breaks here (https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/Console/Command/Command.php#L660), which is calls by the setName.

Running bin/console --env=test for example returns

In Command.php line 659:
                                                                        
  Warning: preg_match() expects parameter 2 to be string, object given

Copy link
Member

Choose a reason for hiding this comment

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

Please use 3.2.2 which fixes this. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants