[Development] [Releasing] Including QTimeZone in Qt 5.1

[John Layt]

On Tuesday 26 Feb 2013 16:12:07 Thiago Macieira wrote:
> On terça-feira, 26 de fevereiro de 2013 22.42.31, John Layt wrote:

> > I'm struggling to think of something different, I can only think of uglies
> > like UnspecifiedTime, NonspecificTime or NotStandardTimeOrDaylightTime. 
> > We can't use CommonTime as that has a different meaning.
> 
> I see. Well, if you explain it, it might make sense.

I'll try write a good explanation, perhaps that might clarify it for me.  I'll 
also ask at the KDE PIM sprint this weekend.

> > So possibly:
> >  - daylightTimeOffset()
> >  - isDaylightTime()
> >  - isCurrentlyDaylightTime()
> 
> Sounds good. I don't like "currently" (using adverbs in the API sounds
> weird). Is it too expensive to do
> isDaylightTime(QDateTime::currentDateTime()) ?

Currently is now dropped, we'll wait and see if there's any demand later.  It 
would actually be isDaylightTime(QDateTime::currentMSecsSinceEpoch()).

> > > - transition() doesn't make sense in view of
> > > nextTransition() and previousTransition(), it needs a better name.
> > 
> > Perhaps effectiveTransition()?
> 
> You'll have to explain what it is. That's why I don't like it.
> 
> Before this email, I thought transition() just didn't make sense.
> 
> Now it looks like it returns the change that one is supposed to make at a
> given transition, whereas the other two methods return the dates of the
> next/previous transition.
> 
> Either way, sounds bad.

You're right transition() doesn't actually describe what it does, probably 
because it's not actually returning a transition per se.  It's actually all 
the values you would get from calling offset(), standardTimeOffset(), 
abbreviation() etc for a given time in one efficient convenience call.  It 
would be valid to call regardless of whether a Time Zone has Transitions or 
not so it shouldn't have transition in the name at all.  I'll try come up with 
a better name, or switch it to be internal only for now as it's useful there.

The transitions(), nextTransition() and previousTransition() are explicitly 
there to allow a specialised app to navigate through transitions when for 
example calculating recurrences, so the exclusive case would be far more 
common.

> > >  - resetSystemTimeZone() needs a very good explanation.
> > 
> > Hmmm, perhaps reloadSystemTimeZone() or refreshSystemTimeZone() would be
> > clearer?
> 
> I see. Sounds extremely internal detail. Do we need it? QDateTime does
> tzset() all the time, so I suggest we always reload.

The thing with caching the System time zone is to prevent unexpected time zone 
changes.  If the system time zone was to dynamically change then an app could 
be left in an inconsistent state, with QDateTime instances having changed but 
gui strings and other cached details or calculations still reflecting the old 
system time zone.  My thinking was that we would have a QEvent::TimeZoneChange 
that the app would respond to by calling the reset.

Perhaps I'll leave the System and Default stuff out of the public api for now 
until we have a clearer idea how this is all going to work.

> > >  - availableTimeZones() for a given offset: maybe unnecessary.
> > 
> Drop it too. Way too specific a use-case.

I'll drop it from the public api for now and keep it private, but I'll ask at 
the KDE PIM sprint this weekend if there's anything that might need it, like 
dealing with iCalendar files.  If not, I'll delete it.

> If you call them "ID", then I'd agree with you. If you're just calling them
> "time zone names", then keep as QString. We could return and accept the
> expanded names of those locations.
> 
> E.g.: America/Sao_Paulo -> "America/São Paulo"

No, they are strictly ID's, they must match the standard Olsen ID's as listed 
by availableTimeZones() or they get rejected.  Lets keep it simple that only 
Olsen ID's are used and not confuse things.  I could have accepted the Windows 
ID's in the constructors too, but didn't for the same reason.  Expanded names 
could be returned by displayName using a new TimeZoneNameType enum value, but 
would probably need a look-up table to be maintained so I won't do that for 
now.

Ideally the ID would actually be an enum, but they change too frequently and 
depend so much on what's installed on a given system that it would cause 
problems.

Cheers!

John.