8000 Add conflict with doctrine/data-fixtures >= 2 by greg0ire · Pull Request #58861 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Add conflict with doctrine/data-fixtures >= 2 #58861

8000 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

Closed
wants to merge 1 commit into from

Conversation

greg0ire
Copy link
Contributor
@greg0ire greg0ire commented Nov 13, 2024
Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues explanation below
License MIT

While working on allowing v2 of doctrine/data-fixtures in the bundle, I stumbled upon an issue that only affects some versions of Symfony that still have a ContainerAwareLoader class.

The signature of ContainerAwareLoader::addFixture() is not compatible with the v2 signature of the Loader interface from doctrine/data-fixtures, as per this fatal error:

Declaration of Symfony\Bridge\Doctrine\DataFixtures\ContainerAwareLoader::addFixture(Doctrine\Common\DataFixtures\FixtureInterface $fixture)
             must be compatible with Doctrine\Common\DataFixtures\Loader::addFixture(Doctrine\Common\DataFixtures\FixtureInterface $fixture): void

The class is not final, which means adding the return type declaration would be a breaking change.

Let us add a conflict to signal this temporary incompatibility. Once this is merged up, the conflict can be bumped to v3 in branches where ContainerAwareLoader no longer exists. Once enough versions of Symfony with this fix are released, I will be able to use a version constraint on symfony/doctrine-bridge that will avoid the CI failure I experienced in the bundle.

The signature of ContainerAwareLoader::addFixture() is not compatible with the
v2 signature of the Loader interface from doctrine/data-fixtures, as per this
fatal error:

> Declaration of Symfony\Bridge\Doctrine\DataFixtures\ContainerAwareLoader::addFixture(Doctrine\Common\DataFixtures\FixtureInterface $fixture)
>              must be compatible with Doctrine\Common\DataFixtures\Loader::addFixture(Doctrine\Common\DataFixtures\FixtureInterface $fixture): void

The class is not final, which means adding the return type declaration would be
a breaking change.

Let us add a conflict to signal this temporary incompatibility.
@derrabus
Copy link
Member

We should try to make the class compatible rather than introducing that conflict.

@greg0ire
Copy link
Contributor Author

@derrabus here is an attempt at doing so: #58865

nicolas-grekas added a commit that referenced this pull request Nov 20, 2024
…v2 (greg0ire)

This PR was merged into the 5.4 branch.

Discussion
----------

Dynamically fix compatibility with doctrine/data-fixtures v2

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | see explanation below
| License       | MIT

While working on [allowing v2 of doctrine/data-fixtures in the bundle](doctrine/DoctrineFixturesBundle#457), I stumbled upon an issue that only affects some versions of Symfony that still have a `ContainerAwareLoader` class.

The signature of `ContainerAwareLoader::addFixture()` is not compatible with the v2 signature of the `Loader` interface from `doctrine/data-fixtures`, as per this fatal error:

```
Declaration of Symfony\Bridge\Doctrine\DataFixtures\ContainerAwareLoader::addFixture(Doctrine\Common\DataFixtures\FixtureInterface $fixture)
             must be compatible with Doctrine\Common\DataFixtures\Loader::addFixture(Doctrine\Common\DataFixtures\FixtureInterface $fixture): void
```

Classes that extend ContainerAwareLoader have to also extend Loader, meaning this is no breaking change because it can be argued that the incompatibility of the extending class would be with the Loader interface.

Closes #58861, Closes #58863

Commits
-------

1812aaf Dynamically fix compatibility with doctrine/data-fixtures v2
@greg0ire greg0ire deleted the add-fixtures-conflict branch November 20, 2024 13:08
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.

4 participants
0