search

makeusabrew / phpperf

PHP Performance Metrics
http://phpperf.com
36 stars 7 forks source link

fix(php results template): php date() function required a timezone set #2

Closed hkdobrev closed 11 years ago

hkdobrev commented 11 years ago

The PHP template used to generate the HTML file with the results from the profiles uses the date() function. My PHP installation requires a timezone to be set. Otherwise it outputs a warning into the HTML code.

I have set the Europe/London as default as the author of the project is based in the UK.

makeusabrew commented 11 years ago

Hey :)

Sorry for such a slow response; GitHub doesn't seem to notify me of PRs / issues a lot of the time so I've missed all three of your PRs. I'm hesitant to accept this one simply because I'm sure I remember reading a while ago (though I can't find any links at the moment) that date_default_timezone_set() was discouraged in favour of setting a timezone in your php.ini - hence I've long since removed it from all of my PHP projects in favour of the ini setting instead. Let me know if this isn't possible on your installation for any reason and I'll have a think about it :)

hkdobrev commented 11 years ago

Well, date_default_timezone_set() is indeed discouraged, because it set the timezone on the fly in the runtime. But projects which are distributable and especially open-source could be run on different environments. That's why some PHP frameworks set the timezone by default so the execution would not fail if you haven't set the timezone yourself.

I accept your point. Though if you just download the phpperf source and run it it won't work on some installations. So I thought this change won't do much bad things, but is a good precaution.

BTW, I ran this in vagrant with php installation from the default chef cookbooks from Opscode. They are not the latest version and they do not have the timezone set. I don't know how common this is though.