[Development] QTimeZone available for review

[Mark]

On Thu, Mar 7, 2013 at 11:10 AM, John Layt <jlayt at kde.org> wrote:
> Hi,
>
> I've finally pushed my proposed QTimeZone support to Gerrit for initial review
> for possible inclusion in 5.1.
>
> The reviews are:
>
> QLocale - Add private countryToCode() method
> https://codereview.qt-project.org/50064
>
> QDateTime - Improve and expose Qt::OffsetFromUtc
> https://codereview.qt-project.org/50065
>
> QDateTime - Extend fromMSecsSinceEpoch API
> https://codereview.qt-project.org/50066
>
> QTimeZone - Define new class and api
> https://codereview.qt-project.org/50067
>
> QTimeZone - Add ICU support
> https://codereview.qt-project.org/50068
>
> QTimeZone - Add TZ File Backend
> https://codereview.qt-project.org/50069
>
> QTimeZone - Add Mac backend
> https://codereview.qt-project.org/50070
>
> QTimeZone - Add Windows backend
> https://codereview.qt-project.org/50071
>
> WIP QDateTime - Add QTimeZone support
> https://codereview.qt-project.org/50072
>
> I'd appreciate particular attention to the API review, and the Mac and Windows
> backends which are not my usual dev environment.
>
> What remains to be done:
> * The QDateTime integration is not quite finished and so marked as WIP, but
> the API changes need review.
> * The Windows backend can have support for historic data added from Vista
> onwards.
> * Lots more test cases
> * A decision on parse/format (see below)
>
> One big change since I first posted the code is there is now no "system time
> zone" that tracks the underlying system time zone as we already provide this
> facility in QDateTime using Qt::LocalTime.
>
> The main issue outstanding is parse/format.  Our current support for time
> zones is inconsistent, asymmetrical, and broken in places.  You can format
> time zones in QLocale and QTime (albeit often the wrong name) but not
> QDateTime, and not parse them anywhere, so round-trip using LongFormat is
> currently broken.  The  existing code is messy, and while format is fairly
> easy to make work, parsing is near impossible to support in our current code.
> It would probably require major changes to the complex and fragile
> QDateTimeParser and QDateTimeEdit classes, but would still be deeply ambiguous
> due to our only supporting a format code for abbreviations.  Full support
> would require either breaking the current behaviour or implementing all new
> code and api names, which seems pointless for just 5.1 when I hope to have the
> new ICU based parser ready for 5.2. Instead I propose to:
>
> * Remove the QDateTime formatter which doesn't support tz and use the QLocale
> formatter instead which does support tz
> * Modify the QLocale formatter to properly format time zones, including the
> Win/Mac system locales
> * To not support parsing of time zones in the old code and clearly document as
> such, recommending apps use a combo-box instead, or use the Qt::ISODate which
> does support parsing of an offset.
> * To leave parsing support until 5.2 using ICU
>
> I realise that might be controversial, and I have had some feedback that this
> is a blocker issue, but I feel the benefits of having most of the Time Zone
> support in 5.1 far out-weighs the few use-cases requiring parsing, especially
> as the existing behaviour is already asymmetrical and broken.
>
> Cheers!
>
> John.

Hi John,

Let me just congratulate you on this quite impressive milestone.
Congrats and keep up the awesome work!

Cheers,
Mark