[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