[Development] [Releasing] Including QTimeZone in Qt 5.1

[Thiago Macieira]

On terça-feira, 26 de fevereiro de 2013 22.42.31, John Layt wrote:
> 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?

I prefer separate commits, but Ossi generally disagrees.

> 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.

I see. Well, if you explain it, it might make sense.

> >  - "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()?

Sounds good.

> >  - 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".

And whether there is any saving of power at all due to those clock shifts :-)

> 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()) ?

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

standardTimeOffset() imples, in my mind, that there is a daylightTimeOffset(). 
If there is, add both. If there isn't, utcOffset() sounds better. I don't think 
there's confusion. In fact, I think it's good to reuse similar terms.

> > - 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.

> 
> 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?

Choose one (inclusive or exclusive) and document it. I'd choose inclusive and 
let people add one second or one millisecond or one day to get the next one.

That does not answer the question about "transition".

> >  - 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?

I don't have enough context to answer that. All I got is "there is no good 
reason to have createTimeZone", so drop that.

> >  - 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.

> >  - static isValid() needs a better name.
> 
> Could be isAvailableId() or isAvailableTimeZone() or isValidId()?

isTimeZoneAvailable() sounds better.

> >  - 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 :-)

Then let's keep the feature off until we get QLocale figured.

> >  - 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.

Drop it too. Way too specific a use-case.

> 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?

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"

> 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).

Keep the feature off for the first version. Let's discuss it after the main 
codebase goes in.

> 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.

Ditto.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.qt-project.org/pipermail/development/attachments/20130226/2d3acb34/attachment.sig>