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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
