Skip to content

Conversation

mpodolian
Copy link

This fixes issue #119.
No unit test, as ini_set / ini_get functions are hard to mock.

@mnapoli
Copy link
Member

mnapoli commented Oct 10, 2015

Hi, thank you for contributing!

This is a very clean PR but I think it might be better to put the date_default_timezone_set() call directly in bin/couscous. The reason is that it's just a simple system call (so it's fine if it's hardcoded somewhere, as you said we don't test it), and also (mainly) because there might be some code executed before this step that might raise the issue.

@mpodolian
Copy link
Author

Agreed, setting it up in the Step felt weird, as it is not project related.

@mnapoli mnapoli added this to the 1.4 milestone Oct 11, 2015
@mnapoli
Copy link
Member

mnapoli commented Oct 11, 2015

Thanks!

mnapoli added a commit that referenced this pull request Oct 11, 2015
Add a configuration step to check that the default timezone is set
@mnapoli mnapoli merged commit 3a7e195 into CouscousPHP:master Oct 11, 2015
@mnapoli
Copy link
Member

mnapoli commented Nov 1, 2015

This has been released as 1.3.2 thank you!

@mnapoli mnapoli mentioned this pull request Nov 1, 2015
@mnapoli mnapoli removed this from the 1.4 milestone Dec 30, 2015
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.

3 participants