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

Fixed bug #72096 Swatch time value incorrect for dates before 1970 #1931

Closed
wants to merge 2 commits into
base: master
from

Conversation

4 participants
@mcq8
Contributor

mcq8 commented Jun 3, 2016

@nikic

This comment has been minimized.

Member

nikic commented Jun 6, 2016

/cc @derickr

@nikic nikic added the Bugfix label Jun 6, 2016

}
?>
--EXPECT--
Time:2016-04-22 00:00:00 = 041 Time:1934-03-04 00:00:00 = 041

This comment has been minimized.

@derickr

derickr Jun 6, 2016

Contributor

I'd change the test to do the following:

  • output the pre-1970 one on the next line
  • explicityly set the timezone in an --INI-- setting to UTC
  • Not use a constant like "25920000000" but instead do "365 * 24 * 3600" (etc) to show where it comes from
  • Add a : between the Bug #72096 and Swatch time in the title
Time:2016-04-22 15:33:20 = 689 Time:1934-03-04 15:33:20 = 689
Time:2016-04-22 17:46:40 = 782 Time:1934-03-04 17:46:40 = 782
Time:2016-04-22 20:00:00 = 875 Time:1934-03-04 20:00:00 = 875
Time:2016-04-22 22:13:20 = 967 Time:1934-03-04 22:13:20 = 967

This comment has been minimized.

@derickr

derickr Jun 6, 2016

Contributor

Last line should also have a new line

@derickr

This comment has been minimized.

Contributor

derickr commented Jun 6, 2016

I've added some comments.

@mcq8

This comment has been minimized.

Contributor

mcq8 commented Oct 9, 2016

@derickr any feedback on this?

@derickr

This comment has been minimized.

Contributor

derickr commented Oct 10, 2016

LGTM

@krakjoe

This comment has been minimized.

Member

krakjoe commented Jan 3, 2017

@derickr can I ask that you personally merge this into appropriate branches as you see fit, please: While we don't have tests that rely on the old behaviour, it's not impossible that someone does have code that does, I would suppose (and I may be wrong).

@nikic

This comment has been minimized.

Member

nikic commented Mar 9, 2017

Merged via b224e74 into PHP 7.0+. Thanks!

@nikic nikic closed this Mar 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment