Review Request: [PATCH 1/4] kcalendarsystem: Fix lengthOfWeek to 7

John Layt jlayt at kde.org
Thu Dec 13 08:48:38 UTC 2012


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


Please do not replace named variables or method calls with "Magic Numbers", this is bad coding practise as it makes the code harder to read and understand and maintain.  If you must optimise the code, please use a "#define DAYS_IN_WEEK 7" instead.  Please do not deprecate daysInWeek() as it is an api method in QDate in Qt5 and we want to maintain a close api match.

- John Layt


On Dec. 13, 2012, 7:41 a.m., Jon Severinsson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107687/
> -----------------------------------------------------------
> 
> (Updated Dec. 13, 2012, 7:41 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> Week length depends on the week system (i.e. KLocale::WeekNumberSystem) used,
> not the calendar used, and is 7 for all week systems supported by KDE.
> 
> 
> Diffs
> -----
> 
>   kdecore/date/kcalendarsystem.h efddd08 
>   kdecore/date/kcalendarsystem.cpp 1aed791 
>   kdecore/date/kcalendarsystemcoptic.cpp 0a95dbe 
>   kdecore/date/kcalendarsystemcopticprivate_p.h 35907ff 
>   kdecore/date/kcalendarsystemgregorian.cpp 40db17a 
>   kdecore/date/kcalendarsystemgregorianprivate_p.h fb7a0dd 
>   kdecore/date/kcalendarsystemhebrew.cpp e3d1484 
>   kdecore/date/kcalendarsystemindiannational.cpp 7222722 
>   kdecore/date/kcalendarsystemislamiccivil.cpp eea98e2 
>   kdecore/date/kcalendarsystemjalali.cpp 889a060 
>   kdecore/date/kcalendarsystemjulian.cpp 9bfc5f9 
>   kdecore/date/kcalendarsystemprivate_p.h d935ead 
>   kdecore/date/kcalendarsystemqdate.cpp f233d219 
>   kdecore/date/kdatetimeparser.cpp a416808 
>   kdecore/date/klocalizeddate.h 1bfe261 
>   kdecore/localization/klocale.h c21bb37 
>   kdecore/localization/klocale_kde.cpp 5409610 
> 
> Diff: http://git.reviewboard.kde.org/r/107687/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jon Severinsson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20121213/d60c26bb/attachment.html>


More information about the Kde-frameworks-devel mailing list