search

sapioit / phpliteadmin

Automatically exported from code.google.com/p/phpliteadmin
1 stars 0 forks source link

remove date_default_timezone_set #186

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
that's not a good tone to set timezone for people.

p.s.
it doesn't save from warning, in fact, it generates it

Original issue reported on code.google.com by ykoro...@gmail.com on 5 Mar 2013 at 9:12

Attachments:

GoogleCodeExporter commented 9 years ago 
Hmm. I agree it's kind of useless at the moment and not much help. But it's not 
true that it generates the error. It moves it.

So normally, people should specify ini-setting date.timezone and then 
everything is fine. If people don't and we do date() for example, this warning 
is produced:

PHP Warning:  date(): It is not safe to rely on the system's timezone settings. 
You are *required* to use the date.timezone setting or the 
date_default_timezone_set() function. In case you used any of those methods and 
you are still getting this warning, you most likely misspelled the timezone 
identifier. We selected the timezone 'UTC' for now, but please set 
date.timezone to select your timezone. in ... phpliteadmin.php on line 1924

The same thing is produced when we do date_default_timezone_get() in this case. 
So in fact, this line does not remove the warning. But it also does not produce 
it, as otherwise, it would come up later. So it only moves it up (to a place 
before $debug enables STRICT messages, so I guess that's why somebody thought 
this was a clever idea). But this is also before we disable display_errors, so 
if strict errors are enabled and displayed, this would in fact move it to a 
place where it is shown.

Well, if we would like to really get rid of any warning, we could do:
date_default_timezone_set(@date_default_timezone_get());

So the @ would hide the warning error and the set would avoid any further 
messages.

But the question is if this is a good idea. Probably not.

I think we should instead add an optional config variable that allows the user 
to set the timezone manually. So if he set the ini setting and that's the 
correct timezone, everything is fine. If not, he can manually set the timezone 
in the config.

Original comment by crazy4ch...@gmail.com on 7 Mar 2013 at 12:42

GoogleCodeExporter commented 9 years ago 
hi

If people specify date in php.ini - of course it will be fine, what you do
- you just provide empty (UTC) value for date.

Personally, I would remove it at all, I cannot remember any situation that
required separate timezones. But if you want it as option - do as option.

Original comment by ykoro...@gmail.com on 7 Mar 2013 at 6:52

GoogleCodeExporter commented 9 years ago 
You need to set a timezone for example if your hoster is in America (and has 
his timezone in php.ini) but you are in Europe for example. You'd somehow need 
to set the timezone for your scripts, or you would get date/time from another 
timezone.

Okay, but I changed my mind.

We don't make heavy use of date functions. In fact what I found is we only 
display the time when the db was changed last, insert the time in an sql dump 
and use the date as filename for exports. So I think it is not necessary to add 
an additional option only for that.

As we disable display_errors in the current version (if $debug is false), the 
user won't be bothered with error messages. And I think it is good if PHP 
produces warning messages that appear in the log. Hopefully people will read 
their log oneday and realize that they should better specify date.timezone ;-)

So we should remove date_default_timezone_set. Agreed.

I have an idea on how we could make sure the date of the last change of db (and 
export date) is not interpreted wrong because it is in another timezone: We 
could simply append the timezone in which it is displayed.

So instead of date('g:ia \o\n F j, Y',...) we do date('g:ia \o\n F j, Y 
(T)',...).
This will end up in a string like this: "11:23pm on February 10, 2013 (CET)".

I think the "(CET)" won't do any harm to anybody, but this way user can make 
sure the date is not in UTC and they think it is in CET (for example).

Original comment by crazy4ch...@gmail.com on 7 Mar 2013 at 12:00

GoogleCodeExporter commented 9 years ago 
(and we should make the date-strings language-dependent, but that's another 
issue).

Original comment by crazy4ch...@gmail.com on 7 Mar 2013 at 12:02

GoogleCodeExporter commented 9 years ago 
Fixed with r349.

Original comment by crazy4ch...@gmail.com on 7 Mar 2013 at 12:11

GoogleCodeExporter commented 9 years ago 
cool idea with CET

Original comment by ykoro...@gmail.com on 7 Mar 2013 at 4:14