Migrate more classes to PHP 8 syntax #10497
Conversation
| EntityManagerInterface $em, | ||
| ClassMetadata $class, | ||
| private readonly ClassMetadata|null $typeClass, | ||
| Collection $collection, |
There was a problem hiding this comment.
$em and $collection could also use constructor promotion.
There was a problem hiding this comment.
I don't think so: $em is not supposed to be passed as null, but can become null on unserialization. $collection is a property inherited from the parent.
lib/Doctrine/ORM/Query/Printer.php
Outdated
| */ | ||
| public function __construct($silent = false) | ||
| public function __construct(protected $_silent = false) |
There was a problem hiding this comment.
| public function __construct(protected $_silent = false) | |
| public function __construct(protected bool $_silent = false) |
The PR is already labeled as a BC break. Isn't this also a good time to remove the underscore prefix?
There was a problem hiding this comment.
I will do a separate, global PR if you don't mind.
There was a problem hiding this comment.
I think we've discussed similar changes already: I wouldn't switch parameter names to the discouraged underscore notation just for the sake of using CPP.
There was a problem hiding this comment.
Actually I just pushed a version with no underscores so that we can deal with both PRs separately. More simple for everyone.
lib/Doctrine/ORM/Query/Printer.php
Outdated
| */ | ||
| public function __construct($silent = false) | ||
| public function __construct(protected $_silent = false) |
There was a problem hiding this comment.
I think we've discussed similar changes already: I wouldn't switch parameter names to the discouraged underscore notation just for the sake of using CPP.
greg0ire
force-pushed
the
php8-migration
branch
from
2e4d671 to
941292f
Compare
greg0ire
dismissed
SenseException’s stale review
feedback has been addressed.
No description provided.