-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
Test + Fix for #68078 #1653
Test + Fix for #68078 #1653
Conversation
Looks good, but add a test for equality too please, and one for negative timestamps :-) |
1e69ce2
to
01e4ae2
Compare
@derickr I added the test for equality. But test for negative timestamps failed as there seems to be a bug in instantiating negative timestamps from format (returns false in PHP). Setting a timestamp with microseconds with |
php -r 'var_dump(DateTime::createFromFormat("U.u", "1.3")->modify("-5 seconds"));' |
21d9983
to
9dc6b66
Compare
There already is a bug report for 'U' with negative timestamps: https://bugs.php.net/bug.php?id=66836 |
9dc6b66
to
83d4b12
Compare
Build keeps failing on other stuff, how do we deal with that? |
@wjzijderveld The Travis CI build failure doesn't appear to be related to your code.
I wouldn't worry about it. |
Merged, after a rebase and a fix |
Summary: I found this bug earlier in PHP and [fixed it there](php/php-src#1653). After that I checked if it was also in HHVM. It was, so I also tried to fix it here 😄 Depends on an updated version of [hhvm-third-party](hhvm/hhvm-third-party#102). Closes #6888 Reviewed By: paulbiss Differential Revision: D3034526 Pulled By: aorenste fbshipit-source-id: 89a10c6af94002bc1992d73341ca5feeb7a2d1b2
Fixes https://bugs.php.net/bug.php?id=68078 (and partially https://bugs.php.net/bug.php?id=68506)