#2794 revert bc break preventing DateTimeImmutable conversion
#2795
Conversation
…ld STILL work, even if unintentionally
…le date types - reverts unintentional BC break
| @@ -53,7 +53,7 @@ public function convertToDatabaseValue($value, AbstractPlatform $platform) | |||
| return $value; | |||
| } | |||
|
|
|||
| if ($value instanceof \DateTime) { | |||
| if ($value instanceof \DateTime || $value instanceof \DateTimeImmutable) { | |||
There was a problem hiding this comment.
Why not just use DateTimeInterface as #2678 suggested?
There was a problem hiding this comment.
As far as I know (and experienced) the interface is quite safe to be used here. I've bothered @derickr also on Twitter to help us. Because we all know that it's easier to get people's attention by being annoying HAHAHA 😜
|
Because according to @derickr it cannot be relied upon
…On 27 Jul 2017 9:21 PM, "Andreas" ***@***.***> wrote:
***@***.**** approved this pull request.
Looks good, as mentioned: we could check for DateTimeInterface unless you
explicitly don't want to cover any possible future implementations of it.
------------------------------
In lib/Doctrine/DBAL/Types/DateTimeType.php
<#2795 (comment)>:
> @@ -53,7 +53,7 @@ public function convertToDatabaseValue($value, AbstractPlatform $platform)
return $value;
}
- if ($value instanceof \DateTime) {
+ if ($value instanceof \DateTime || $value instanceof \DateTimeImmutable) {
Why not just use DateTimeInterface as #2678
<#2678> suggested?
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#2795 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakMsXgibF-hyIn6HYhXVWBxLYv9Rfks5sSNU9gaJpZM4Ololw>
.
|
IIRC, there was something about it not being a true interface in the sense of "you can typehint against it but we don't want you to implement it" (see note in docs). Doesn't matter, fix looks good as is, was just wondering. |
|
As far as I know, DateTimeInterface exists just for this purpose, I don't really understand why it would be a good idea to use it. Actually typehinting is the only way it's good for - it can't be implemented in user-land anyway. And official documentation nods:
|
…ateTimeInterface` instead
Reverts the unintentional BC break reported in #2794