Add DateImmutable type #2118
Add DateImmutable type #2118
Conversation
| @@ -211,11 +212,11 @@ public function executeInserts(array $options = []) : void | |||
| if ($this->class->isVersioned) { | |||
| $versionMapping = $this->class->fieldMappings[$this->class->versionField]; | |||
| $nextVersion = null; | |||
| if ($versionMapping['type'] === 'int') { | |||
| if ($versionMapping['type'] === Type::INT || $versionMapping['type'] === Type::INTEGER) { | |||
There was a problem hiding this comment.
I'm not sure why we haven't used consts in the first place so I've used them everywhere and kept it as a separate commit
There was a problem hiding this comment.
We should deprecate and remove the boolean and integer types to be consistent with PHP, which only allows bool and int. For now this works. 👍
malarzm
force-pushed
the
datetime-immutable-type
branch
3 times, most recently
from
2141a03
to
6fd291e
Compare
malarzm
force-pushed
the
datetime-immutable-type
branch
from
6fd291e
to
98dcc66
Compare
| { | ||
| $datetime = parent::getDateTime($value); | ||
|
|
||
| return $datetime instanceof DateTime ? DateTimeImmutable::createFromMutable($datetime) : $datetime; |
There was a problem hiding this comment.
This can lead to returning something that isn't a DateTimeImmutable if so given from the parent class. I would suggest checking for that.
There was a problem hiding this comment.
DateTimeInterface has only two implementations and IIRC it's forbidden to write a new one in userland. But if you prefer so I can check what's returned and throw an exception if it's neither DateTime or DateTimeImmutable 👍
| @@ -79,14 +79,14 @@ public function testMultipleFlushesDoIncrementalUpdates() | |||
| $this->dm->persist($test); | |||
| $this->dm->flush(); | |||
|
|
|||
| $this->assertIsInt($test->getVersion()); | |||
| $this->assertInternalType('int', $test->getVersion()); | |||
There was a problem hiding this comment.
Why the change? This method is deprecated in PHPUnit 8.
There was a problem hiding this comment.
Somehow I had newest PHPUnit running and it complained while I was running entire LockTest class. Can revert it and keep the change for the upgrade PR 👍
There was a problem hiding this comment.
Yes please, let's refactor that separately.
malarzm
force-pushed
the
datetime-immutable-type
branch
2 times, most recently
from
495abc4
to
2e74793
Compare
malarzm
force-pushed
the
datetime-immutable-type
branch
from
2e74793
to
a1590d0
Compare
Summary
As per title :)