Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

Adds time zone support for provider methods returning DateTime instance #675

Merged
merged 3 commits into from
Apr 29, 2016
Merged

Conversation

bishopb
Copy link
Contributor

@bishopb bishopb commented Aug 19, 2015

There are four DateTime returning methods: dateTime, dateTimeAD, dateTimeBetween, and dateTimeInterval, Two of these (dateTimeBetween and dateTimeInterval) set the timezone on the returned object, while the other two do not. This can lead to user land juggling timezones when comparing dates returned from the different generators.

This PR updates these four DateTime returning methods to set the time zone to result of date_default_timezone_get or the user-specified timezone given in the final argument to all methods. Test coverage updated to check that the default and custom time zones are handled.

@fzaninotto
Copy link
Owner

This is a BC break, because dates that were using date_default_timezone_get() now use UTC by default. Please consider using date_default_timezone_get() as the default, or I'll have to close the PR.

… result of date_default_timezone_get() for all methods which return a \DateTime.
@bishopb
Copy link
Contributor Author

bishopb commented Feb 28, 2016

@fzaninotto Ah, yes. Fixed and tests passing.

@fzaninotto
Copy link
Owner

Great! One thing is missing though: could you update the part of the Readme that documents the formatters you've modified to add the new parameter?

@bishopb
Copy link
Contributor Author

bishopb commented Feb 29, 2016

@fzaninotto Done, but looking over the code again, I think the other DateTime methods should also take $timezone to be consistent. Specifically dateTimeThisCentury, dateTimeThisDecade, dateTimeThisYear, and dateTimeThisMonth.

What do you think?

@fzaninotto fzaninotto merged commit c392ecd into fzaninotto:master Apr 29, 2016
@fzaninotto
Copy link
Owner

Thanks a lot!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants