-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Update DateFormat.php to fix deprecated method call: PHP >= 5.5.0. #5295
Conversation
In versions of PHP >= 5.5.0, the method IntlDateFormatter::setTimeZoneId() and IntlDateFormatter::setTimeZone() should be used instead. This is my first proposed zf fix, so hopefully the ternary + dynamic method name var is acceptable.
Is it need to do a runtime check for this? I suggest let this as is until ZF reach minimum PHP 5.5 |
Well, I'll put it this way, I'm using PHP5.5 and I ended up writing my own solution instead of using this class because every time I tried to use it it would give PHP notices. Also, since the Travis system that runs the unit tests wasn't failing on this during the php5 tests before, its likely that the unit tests aren't testing all scenarios. Sadly my knowledge ends there or I'd dig more into why that is. |
@@ -133,8 +133,11 @@ public function setTimezone($timezone) | |||
{ | |||
$this->timezone = (string) $timezone; | |||
|
|||
// The method setTimeZoneId is deprecated as of PHP 5.5.0 | |||
$setTimeZoneMethodName = version_compare(PHP_VERSION, '5.5.0', '<') ? 'setTimeZoneId' : 'setTimeZone'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change against PHP_VERSION*
constants (http://www.php.net/manual/reserved.constants.php#reserved.constants.core)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hickeroar yes; as @marc-mabe said use the PHP_VERSION constants; the version_compare has odd results due to distribution specific areas and can cause issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but I'm not sure I understand what you mean. I copied the version_compare line from earlier in the file when I created this line. Link: https://github.com/zendframework/zf2/blob/master/library/Zend/I18n/View/Helper/DateFormat.php#L93
Can you explain what I should change about it? Sorry if that's a dumb question. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should write if (PHP_VERSION_ID < 50500) {
because using version_compare()
is much slower and the constant PHP_VERSION
can contain some special version suffixes the function could be incompatible with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we can let as is and later @marc-mabe can send a PR improving the performance along all the framework.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update DateFormat.php to fix deprecated method call: PHP >= 5.5.0.
- Use more reliable version comparison
- use PHP_VERSION_ID always - comparison should be against integers
Forward port #5295 Conflicts: library/Zend/I18n/View/Helper/DateFormat.php
I incorporated the feedback on the version comparisons when merging. |
…tch-1 Update DateFormat.php to fix deprecated method call: PHP >= 5.5.0.
- Use more reliable version comparison
- use PHP_VERSION_ID always - comparison should be against integers
Forward port zendframework/zendframework#5295 Conflicts: library/Zend/I18n/View/Helper/DateFormat.php
In versions of PHP >= 5.5.0, the method IntlDateFormatter::setTimeZoneId() is deprecated and IntlDateFormatter::setTimeZone() should be used instead.
This is my first proposed zf fix, so hopefully the ternary + dynamic method name var is acceptable.