Conversation
lib/Doctrine/ODM/MongoDB/Tools/Console/Command/Schema/AbstractCommand.php
Outdated
Show resolved
Hide resolved
GromNaN
force-pushed
the
symfony8
branch
3 times, most recently
from
d144a24 to
5781c4a
Compare
GromNaN
force-pushed
the
symfony8
branch
3 times, most recently
from
e73943c to
230d603
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for Symfony 8.0 by implementing compatibility layers for console command configurations that now require return type declarations. The main change addresses Symfony 8.0's requirement for the configure() method to have a void return type.
- Introduces compatibility traits to handle different Symfony versions' return type requirements for console commands
- Updates dependency constraints to include Symfony 8.0 support
- Adds CI testing for Symfony 8.0 with conditional dependency handling
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| composer.json | Updates Symfony dependencies to support version 8.0, excludes symfony/var-exporter 8.0 due to lazy ghost removal |
| lib/Doctrine/ODM/MongoDB/Tools/Console/Command/CommandCompatibility.php | Adds Symfony 8.0 compatibility with return type declarations for configure() method |
| lib/Doctrine/ODM/MongoDB/Tools/Console/Command/Schema/AbstractCommandCompatibility.php | New trait providing configure() method compatibility for schema commands |
| lib/Doctrine/ODM/MongoDB/Tools/Console/Command/Schema/AbstractCommand.php | Refactors to use compatibility trait and renames configure to configureCommonOptions |
| Various command files | Updates configure() method signatures to use doConfigure() pattern for compatibility |
| .github/workflows/continuous-integration.yml | Adds Symfony 8.0 testing with conditional dependency installation |
| phpcs.xml.dist | Excludes new compatibility file from PSR1 multiple classes rule |
lib/Doctrine/ODM/MongoDB/Tools/Console/Command/Schema/DropCommand.php
Outdated
Show resolved
Hide resolved
GromNaN
force-pushed
the
symfony8
branch
2 times, most recently
from
bf00ddc to
92ad1f8
Compare
| protected function configure(): void | ||
| { | ||
| $this->doConfigure(); | ||
| } |
There was a problem hiding this comment.
Perhaps I'm missing something but I don't think we need this trait here.
Since we already require PHP 8.1+, we should be able to use void in child classes as per: https://3v4l.org/Mv3oq
I did something similar in https://github.com/IonBazan/composer-diff/blob/8907711f0bf74677041f7b356da2367afea44a53/src/Command/BaseTypedCommand.php#L9-L14 but that's only needed to support different PHP versions.
If the parent does not declare a return type then the child is allowed to declare one.
There was a problem hiding this comment.
We can remove it in 2.13.x just to be sure
There was a problem hiding this comment.
The issue is with older versions of the bundle that don't have the void return type. https://github.com/doctrine/DoctrineMongoDBBundle/blob/4.7.x/Command/CreateSchemaDoctrineODMCommand.php#L21
GromNaN
force-pushed
the
symfony8
branch
2 times, most recently
from
7348894 to
0509a18
Compare
Summary
Dependencies blocking Symfony upgrade:
doctrine/ormRun tests with Symfony 8 orm#12102friendsofphp/proxy-manager-ltsphpbench/phpbenchSupport for Native Lazy objects is required, as Symfony 8.0 drops support for lazy ghost objects. This version will never be compatible with
symfony/var-exporter:8