[Development] [Releasing] Including QTimeZone in Qt 5.1

[John Layt]

On Wednesday 20 Feb 2013 16:16:55 Thiago Macieira wrote:

> For 12 commits, I'd just submit straight to dev.

Would you prefer it squashed to one big commit, or keep the platform backends 
separate commits?

Except as noted below all other suggested changes are being made.

>  - GenericTime and StandardTime are too close in name.

Both Mac and ICU use these terms as defined in CLDR, so no hints there.  
Standard/Daylight are the most commonly used names, including on Windows, so 
Generic must change. Usage examples are:

Standard : Pacific Standard Time
Daylight : Pacific Daylight Time
Generic  : Pacific Time

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.

>  - "name()" is too simple, but there's precedent (QLocale::name())

QLocale::name() is really the code, so best to avoid it.  ICU and Win use 
displayName() and Mac uses localizedName().

So displayName()?

>  - I'm wondering if we should expand "DST" - dstOffset, isDst,
>    isCurrentlyDst 

Mac and Windows use "DaylightSavingTime", ICU uses "DaylightTime".  Not using 
the Saving is shorter and avoids arguments over "Saving" verses "Savings".

So possibly:
 - daylightTimeOffset()
 - isDaylightTime()
 - isCurrentlyDaylightTime()

We'd change the enum to match.

I wonder if utcOffset() might then be more consistent as standardTimeOffset()?  
Might save confusion with the QDateTime OffsetFromUtc.

> - transition() doesn't make sense in view of
> nextTransition() and previousTransition(), it needs a better name.

Perhaps effectiveTransition()?

Or switch it around to transitionForTime() and transitionAfterTime() and 
transitionBeforeTime().

Or ICU just has the one method that takes an enum for TransitionType of 
Before, BeforeInclusive, After and AfterInclusive?

>  - do we need createTimeZone()?

I wasn't sure if we'd hit a scenario where factory methods with different 
names but the same parms would give better control over what backend gets 
used.  Now I can't really think of a scenario where the constructor wouldn't 
work, other than point 2) below.  For the general case constructors would make 
for simpler client code, and we can add custom factory methods if really 
needed later.

So change to be constructors instead?

>  - resetSystemTimeZone() needs a very good explanation.

Hmmm, perhaps reloadSystemTimeZone() or refreshSystemTimeZone() would be 
clearer?

>  - static isValid() needs a better name.

Could be isAvailableId() or isAvailableTimeZone() or isValidId()?

>  - availableTimeZones() for QString: maybe one of the QLocale enums?

We'd have to change regionCode() to return it as well.  We'd need to have 
internal code conversion routines, we do have QLocalePrivate::codeToCountry() 
but we'd need a new countryToCode().  

It would be nice to expose the conversion routines as public api in QLocale 
for anyone who only has the ISO code, it's what the outside world uses :-)

>  - availableTimeZones() for a given offset: maybe unnecessary.

It's in the ICU api and I've seen it used a couple of places to help choose a 
time zone, i.e. you know the offset is +1 hour so what are the possible 
options.  That said Mac and KDE don't offer it so I wouldn't really miss it.


A couple of general questions:

1) The Olsen ID's are defined to only ever be ASCII, so we could make the API 
use a QByteArray instead, but QString just seems more future proof to me?

2) I'm trying to think of a nice way to allow an app on Windows to choose to 
use the ICU or TZ File backend instead of the Windows backend, i.e. a PIM app 
may want more accurate and consistent conversions.  I initially thought to 
have methods like createTzTimeZone() and availableTzTimeZones() that they 
called when they wanted instances, but that doesn't make for very portable 
code.  An alternative would be to allow them to set the System Time Zone, but 
I could see confusion then with the default time zone (they're separate 
currently with the system being immutable so that the availableTimeZones() 
list always returns the system list, for example in case you set the default 
to be a UTC offset instance which you then don't want used for the list).

3) I'm also thinking about api to load a TZ File from a given QFileInfo, but 
I'm not sure the the use or wisdom of doing that.

Cheers!

John.