Symfony 8 support#44
Conversation
|
Cloud someone merge this and release a new version please? @maximecolin @benji07 @Tom32i |
There was a problem hiding this comment.
Thanks a lot for the PR 🙏 The migration is clean and consistent (Extension namespace fix, DI configs moved to PHP, composer/CI bumps). A few remarks to address before merging — especially points 1 and 2 in the inline comments; the rest can be follow-ups.
Also: dropping Symfony 7.0–7.3 support (keeping only ^6.4 and ^7.4 LTS + ^8.0) is worth mentioning explicitly in the release notes of the next version. And the README is outdated (Symfony 4.0 badge, AppKernel.php instructions) — pre-existing debt, but this would be a good time to refresh it.
|
|
||
| /** | ||
| * @param array<string,mixed> &$vars | ||
| * @param array<mixed> &$vars |
There was a problem hiding this comment.
This downgrade from array<string,mixed> to array<mixed> looks like an unnecessary loss of precision: $vars receives $view->vars, which is string-keyed. It also creates an inconsistency with optionEquals() just below, which keeps array<string,mixed>. Could you revert to array<string,mixed>? (and if phpstan was complaining, it's worth understanding why)
| @@ -30,7 +30,7 @@ protected function setUp(): void | |||
| /** | |||
| * Get Form Type Extensions | |||
| * | |||
| * @return array<FormTypeExtensionInterface> | |||
| * @return array<FormTypeExtensionInterface<Extension\TreeAwareExtension>> | |||
There was a problem hiding this comment.
FormTypeExtensionInterface is not generic in Symfony core, so the template parameter <Extension\TreeAwareExtension> is semantically empty (Lint passes, so not fatal, but misleading). The previous array<FormTypeExtensionInterface> was clearer — shall we revert to it?
| uses: shivammathur/setup-php@v2 | ||
| with: | ||
| php-version: '8.3' | ||
| php-version: '8.2' |
There was a problem hiding this comment.
Linting is usually run on the highest supported PHP version. Is this change from 8.3 to 8.2 intentional? (8.3+ would catch recent deprecations better)
There was a problem hiding this comment.
Setting PHP_CS_FIXER_IGNORE_ENV environment variable is deprecated and will be removed in 4.0, use unsupportedPhpVersionAllowed config instead.
You are running PHP CS Fixer on PHP 8.5.7, but the minimum PHP version supported by your project in composer.json is PHP 8.2. Executing PHP CS Fixer on newer PHP versions may introduce syntax or features not yet available in PHP 8.2, which could cause issues under that version. It is recommended to run PHP CS Fixer on PHP 8.2, to fit your project specifics.
If you need help while solving warnings, ask at https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/discussions/, we will help you!
| 'simplified_null_return' => false, | ||
| 'void_return' => true, | ||
| 'yoda_style' => [], | ||
| 'yoda_style' => ['equal' => false, 'identical' => false, 'less_and_greater' => false], |
There was a problem hiding this comment.
Disabling yoda_style is a style change unrelated to SF8 support (scope creep). Nothing blocking, but it would ideally belong in a dedicated commit/PR.
There was a problem hiding this comment.
It was required by php-cs-fixer to configure it explicitly.
1ed
left a comment
There was a problem hiding this comment.
Thanks for the fast review.
| uses: shivammathur/setup-php@v2 | ||
| with: | ||
| php-version: '8.3' | ||
| php-version: '8.2' |
There was a problem hiding this comment.
Setting PHP_CS_FIXER_IGNORE_ENV environment variable is deprecated and will be removed in 4.0, use unsupportedPhpVersionAllowed config instead.
You are running PHP CS Fixer on PHP 8.5.7, but the minimum PHP version supported by your project in composer.json is PHP 8.2. Executing PHP CS Fixer on newer PHP versions may introduce syntax or features not yet available in PHP 8.2, which could cause issues under that version. It is recommended to run PHP CS Fixer on PHP 8.2, to fit your project specifics.
If you need help while solving warnings, ask at https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/discussions/, we will help you!
| @@ -30,7 +30,7 @@ protected function setUp(): void | |||
| /** | |||
| * Get Form Type Extensions | |||
| * | |||
| * @return array<FormTypeExtensionInterface> | |||
| * @return array<FormTypeExtensionInterface<Extension\TreeAwareExtension>> | |||
|
|
||
| /** | ||
| * @param array<string,mixed> &$vars | ||
| * @param array<mixed> &$vars |
| 'simplified_null_return' => false, | ||
| 'void_return' => true, | ||
| 'yoda_style' => [], | ||
| 'yoda_style' => ['equal' => false, 'identical' => false, 'less_and_greater' => false], |
There was a problem hiding this comment.
It was required by php-cs-fixer to configure it explicitly.



Adds support for Symfony 8 and removes support for unmaintained versions.