[Development] [Releasing] Including QTimeZone in Qt 5.1

Thiago Macieira thiago.macieira at intel.com
Thu Feb 21 01:16:55 CET 2013


On quarta-feira, 20 de fevereiro de 2013 23.26.51, John Layt wrote:
> Hi,
> 
> As per Lars' email to development, I would like to submit my implementation
> of QTimeZone and its integration in QDateTime for inclusion in Qt 5.1.  My
> development branch is currently on gitorious [1] but I'd like to push it to
> a Gerrit feature branch by the end of this week to get the review process
> under way and open up for contributions.  What's the best process for this?

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

Quick review notes:

 - copyright 2013, not 2012 and please put the email address between < >
 - do not create global static non-PODs. That's what we have Q_GLOBAL_STATIC 
for.
 - please try to make each commit compile on its own
 - QTimeZonePrivate::init() should be private or it should not exist. It must 
not be called from derived classes.
 - use Q_DECL_OVERRIDE in any new code that is doing virtual overrides

API review:

 - GenericTime and StandardTime are too close in name. Please rename one to 
something sematically more distant. Let's not repeat the mistake of 
QMultimedia::MaybeSupported and ProbablySupported.
 - QTimeZone has a virtual destructor, but no virtual methods and it's 
copyable. Make it non-virtual.
 - "name()" is too simple, but there's precedent (QLocale::name())
 - why does name() take a QString for a locale instead of a QLocale?
 - I'm wondering if we should expand "DST" - dstOffset, isDst, isCurrentlyDst
 - transition() doesn't make sense in view of nextTransition() and 
previousTransition(), it needs a better name.
 - do we need createTimeZone()?
 - resetSystemTimeZone() needs a very good explanation.
 - static isValid() needs a better name.
 - availableTimeZones() for QString: maybe one of the QLocale enums?
 - availableTimeZones() for a given offset: maybe unnecessary.


> QTimeZone features implemented:
> * Thread-safe time zone calculator
> * Windows XP system backend
> * Mac system backend
> * TZ File backend
> * ICU backend
> * Partial integration in QDateTime
> 
> Outstanding work:
> * Finish QDateTime integration
> * Finish test cases
> * Finish QDoc
> * Windows Vista system backend
> 
> I expect most of the code required to be completed in teh next few days,
> with just some clean-up work, tests and docs to be finished in the feature
> branch over the next 2 weeks before merging.

Nice work!

-- 
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/20130220/1eef5898/attachment.sig>


More information about the Development mailing list