Deprecate passing timezone information to methods where it is ignored #6020
Conversation
|
@greg0ire, do you think something like this can prevent those foot shots? |
phansys
force-pushed
the
tz_offsets
branch
2 times, most recently
from
84e812e
to
0cdd7a4
Compare
|
There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. |
|
@phansys sorry for ignoring you, I must have get swamped. Yes, I think that would help. |
phansys
force-pushed
the
tz_offsets
branch
3 times, most recently
from
694728b
to
f4767aa
Compare
Don't worry! no problem. we all have our times and own occupations. |
phansys
force-pushed
the
tz_offsets
branch
2 times, most recently
from
bcd0866
to
5260030
Compare
phansys
changed the title
src/Types/DateImmutableType.php
Outdated
| if ($offset !== $defaultOffset) { | ||
| Deprecation::triggerIfCalledFromOutside( | ||
| 'doctrine/dbal', | ||
| 'https://github.com/doctrine/dbal/pull/xxxx', |
There was a problem hiding this comment.
| 'https://github.com/doctrine/dbal/pull/xxxx', | |
| 'https://github.com/doctrine/dbal/pull/6020', |
src/Types/DateImmutableType.php
Outdated
| if ($offset !== $defaultOffset) { | ||
| Deprecation::triggerIfCalledFromOutside( | ||
| 'doctrine/dbal', | ||
| 'https://github.com/doctrine/dbal/pull/xxxx', |
There was a problem hiding this comment.
| 'https://github.com/doctrine/dbal/pull/xxxx', | |
| 'https://github.com/doctrine/dbal/pull/6020', |
phansys
force-pushed
the
tz_offsets
branch
4 times, most recently
from
ed2abda
to
b10717c
Compare
|
Ah I need to merge up. Hang on. |
|
That worked. Please address the remaining CS issues. |
| @@ -117,4 +130,96 @@ public function testRequiresSQLCommentHint(): void | |||
| { | |||
| self::assertTrue($this->type->requiresSQLCommentHint($this->platform)); | |||
| } | |||
|
|
|||
| /** @dataProvider provideDateTimeInstancesWithTimezone */ | |||
| public function testTimezoneDeprecationFromConvertsToDatabaseValue( | |||
There was a problem hiding this comment.
| public function testTimezoneDeprecationFromConvertsToDatabaseValue( | |
| public function testTimezoneDeprecationFromConvertToDatabaseValue( |
| $defaultOffset = (new DateTimeImmutable())->format('O'); | ||
|
|
||
| if ($offset !== $defaultOffset) { | ||
| Deprecation::triggerIfCalledFromOutside( |
There was a problem hiding this comment.
Is there a realistic situation where this deprecation might be triggered? In what case could the DBAL provide timezone information to this method? If it is lost when saving to the database, this seems unlikely, but maybe there are other situations I did not think of? If the timezone of the database server and the application server differ, maybe?
There was a problem hiding this comment.
I'm not sure, I must review the flow in order to answer.
|
There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. |
|
This pull request was closed due to inactivity. |
Summary
The offset comparison is made against the default timezone instead of UTC because the user may be using a specific default timezone on purpose.
Related to #6014 and #6017 (comment).