[Kde-pim] Review Request: Allow Incidence Editor to Handle Additional TimeZones

[Kevin Krammer]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102945/#review7547
-----------------------------------------------------------



incidenceeditor-ng/incidencedatetime.cpp
<http://git.reviewboard.kde.org/r/102945/#comment6530>

    Since you only need the KTimeZone part, why not just store that in a local variable?
    I would also recommend using const to indicate to any reader that it is just a convenience shortcut and not something that is being altered by the method



incidenceeditor-ng/incidencedatetime.cpp
<http://git.reviewboard.kde.org/r/102945/#comment6531>

    Assuming setDateTimes could be called multiple times (e.g. by accident), maybe check if mTimesZones already contains the value?



incidenceeditor-ng/ktimezonecombobox.cpp
<http://git.reviewboard.kde.org/r/102945/#comment6532>

    Rest of the file seems to put else on the same line as the if's closing brace


- Kevin Krammer


On Oct. 23, 2011, 1:40 a.m., David Narváez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102945/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2011, 1:40 a.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> If the incidence has a time zone not included in the system timezones, add it to the TimeZone box.
> 
> 
> Diffs
> -----
> 
>   incidenceeditor-ng/incidencedatetime.cpp ff3fdc3 
>   incidenceeditor-ng/ktimezonecombobox.cpp 7fbfef1 
> 
> Diff: http://git.reviewboard.kde.org/r/102945/diff/diff
> 
> 
> Testing
> -------
> 
> 1. In KOrganizer, add an event with local timezone
> 2. In KOrganizer, add the event located here https://bugs.kde.org/attachment.cgi?id=63679
> 3. Edit both events and notice the Time Zones displayed in the boxes - one is local time zone and the other one is called "Eastern Standard Time" time zone
> 
> You currently need review request 102838 to be able to do step 2 correctly.
> 
> 
> Thanks,
> 
> David Narváez
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/