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

Drop container-interop/container-interop #729

Merged
merged 3 commits into from
Dec 22, 2022
Merged

Drop container-interop/container-interop #729

merged 3 commits into from
Dec 22, 2022

Conversation

tomudding
Copy link
Contributor

As suggested in #728 this drops the abandoned container-interop/container-interop in favour of psr/container. This means that the Interop\Container\ContainerInterface interface is replaced with Psr\Container\ContainerInterface.

Unlike doctrine/DoctrineModule#792 not all changed signatures are in final classes, specifically the CliConfigurator. This may require bumping laminas/laminas-servicemanager to at least v3.11.0, @driehle please advise.

@tomudding
Copy link
Contributor Author

I am aware of the failing tests, I will address these in a bit.

@fezfez fezfez mentioned this pull request Dec 22, 2022
3 tasks
@fezfez
Copy link
Contributor

fezfez commented Dec 22, 2022

seem good to me 🥇

@driehle
Copy link
Member

driehle commented Dec 22, 2022

This may require bumping laminas/laminas-servicemanager to at least v3.11.0, @driehle please advise.

Indeed, we need to bump laminas-servicemanager to the version introducing class aliases for the interop and the Psr containers. For the records, boesing has explained this here: php-fig/container#37

Please bump to ^3.17.0. Otherwise this PR looks fine.

…servicemanager`

This drops the abandoned `Interop\Container\ContainerInterface`
interface in favour of `Psr\Container\ContainerInterface`.

To ensure that the signature change for `CliConfigurator` does
not break anything, we also bump `laminas/laminas-servicemanager`
to `3.17.0` in order to ensure that class aliases for the Interop
and Psr containers exist.
The Laminas `AdapterInterface` is absolutely not restrictive of the type
of the items that can be in the paginator's collection. However, here
the incorrect Psalm type was used.

The `DoctrinePaginator` expects an `object` (typed as `T`), such as
`Doctrine\ORM\Tools\Pagination\Paginator`. However, this `T` is not the
correct type for the items in the paginator.
`CliConfigurator::getHelpers()` should always return.
@tomudding
Copy link
Contributor Author

Please bump to ^3.17.0. Otherwise this PR looks fine.

I have amended my first commit to bump the version to ensure correct history. Thank you for your time taking a look at this :)

@driehle driehle added the Enhacement New feature or request label Dec 22, 2022
@driehle driehle added this to the 5.3.0 milestone Dec 22, 2022
@driehle driehle self-assigned this Dec 22, 2022
@driehle driehle merged commit 676b10c into doctrine:5.3.x Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhacement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants