[Routing] Remove variadic constructor signature #46157
Conversation
|
Can you check if the build failure is related? |
6c47d2f to
6d908cd
Compare
|
Fixed fabbot, the CI test failures are caused by changes in PHP 8.2 upstream. You can see the same errors in e.g. https://github.com/symfony/symfony/runs/6146660539 Btw, let's mark PHP 8.2 tests as experimental again: #46160 |
6d908cd to
eb53789
Compare
|
The benefit of using variadic was the type check enforced by PHP. |
src/Symfony/Component/Routing/Tests/Requirement/EnumRequirementTest.php
Outdated
Show resolved
Hide resolved
👍 or 👎? |
05ade4d to
a04756f
Compare
|
👍 for |
|
It's actually |
da295d9 to
0fdaaa8
Compare
0fdaaa8 to
2183052
Compare
2183052 to
49644c6
Compare
|
Thank you @wouterj. |
| * @var string[] | ||
| */ | ||
| private readonly array $values; | ||
| private string $requirement; |
There was a problem hiding this comment.
@nicolas-grekas Why did you remove the readonly?
There was a problem hiding this comment.
Not intentional! (But read-only on private is not useful either so 🤷♂️ 😆)
Just a little suggestion, based on today's "New in Symfony" blogpost.
Subjective argument: the variadic signature felt a bit weird, as argument 1 has a different meaning than 2+ (but they look very similar due to PHP's
::class"magic constant")Objective argument: variadic signature is harder with BC layers when we want to add another argument. Personally, I feel like we have to be careful about using variadic signatures in Symfony.