Skip to content
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

Get summer/winter time from PHP timezone library #103

Merged
merged 3 commits into from
Mar 17, 2018

Conversation

c960657
Copy link
Contributor

@c960657 c960657 commented Mar 17, 2018

The date of transition to/from daylight saving time, and whether daylight saving time has been used at all, has changed over the years. This information is available in the tz (Olson) database that is included in PHP.

This PR makes Yasumi use the tz database for calculating the transitions. This works for any timezone that observes daylight saving time.

It seems that PHP currently only supports years until 2037 (when Unix timestamps exceed 31 bits), but this will likely be fixed in due time, if daylight saving time exists at all at that time.

@stelgenhof
Copy link
Member

Thanks for the PR! I think it is a good one, since it can now be used by other Holiday Providers as well. Always love a good refactor :)

@stelgenhof stelgenhof self-requested a review March 17, 2018 05:26
Copy link
Member

@stelgenhof stelgenhof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to run the code style script (composer php-cs-fixer). That is why Travis failed :)
Can you also add a changelog entry?

$transition = array_shift($transitions);
$dst = $transition['isdst'];

foreach ($transitions as $transition) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems you rely on the fact that the getTransitions function returns the transitions sorted by time, correct? If returned randomly, I think getting the transitions for summer and wintertime then might not work this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT from the PHP source code, we can rely on the dates being returned in chronological order. It seems the underlying data structure is also sorted by date.

https://github.com/php/php-src/blob/ef255c9f0f2c2ad1ea224ed215edb00f5bc205bd/ext/date/php_date.c#L4013

@c960657
Copy link
Contributor Author

c960657 commented Mar 17, 2018

Travis is happy now :-)

Copy link
Member

@stelgenhof stelgenhof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect!

@stelgenhof stelgenhof merged commit c56cad5 into azuyalabs:develop Mar 17, 2018
@c960657 c960657 deleted the summer-time branch March 19, 2018 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants