-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Yaml] set arguments depending on the PHP version #20408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
xabbuh
commented
Nov 4, 2016
Q | A |
---|---|
Branch? | 3.1 |
Bug fix? | no |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | n/a |
License | MIT |
Doc PR | n/a |
So, regarding #20291 (comment) |
Yeah, I think it doesn't hurt to wait what HHVM will do in the future. We can still adapt the test then if needed. |
@@ -507,7 +507,12 @@ public function testParseTimestampAsDateTimeObject($yaml, $year, $month, $day, $ | |||
$expected = new \DateTime($yaml); | |||
$expected->setTimeZone(new \DateTimeZone('UTC')); | |||
$expected->setDate($year, $month, $day); | |||
@$expected->setTime($hour, $minute, $second, 1000000 * ($second - (int) $second)); | |||
|
|||
if (PHP_VERSION_ID >= 70100) { |
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.
Will this work for different HHVM versions?
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.
Depends on which PHP version they claim to be compatible 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 got recommended to do a feature check where possible instead of a version check. Not sure if this is actually possible without reflection
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.
This is a test case, we don't care...
Thank you @xabbuh. |
This PR was merged into the 3.1 branch. Discussion ---------- [Yaml] set arguments depending on the PHP version | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Commits ------- ad54d83 [Yaml] set arguments depending on the PHP version