[Kde-pim] Review Request: Fix Message Group Aggregation by Date

[Thomas McGuire]

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

Ship it!


Looks good, please commit :)
And thanks again.

- Thomas


On 2010-01-11 12:03:36, John Layt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2521/
> -----------------------------------------------------------
> 
> (Updated 2010-01-11 12:03:36)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> See bug report, message aggregation by date gets confused in January due to use of ISO week numbers.  Dates are also not consistently localised.
> 
> I've assumed we want the week split to start from KLocale::weekStartDay() and not KLocale::workingWeekStartDay() and not by ISO week number which would always be Monday.  
> 
> I've kept the other splits as current, but there does seem to be a little inconsistency, i.e. if GroupByDate as soon as the mail falls into the previous month or week it doesn't get considered for day name except for Yesterday which is always grouped, whereas in GroupByDateGroup if the mail falls in the last 7 days it gets a day name regardless of what week or month it belongs to.  It seems the wrong way around to me.
> 
> I'm also wondering about the widespread use of time_t from time.h, shouldn't we be using something from KDE or Qt instead, or was there some reason not to?
> 
> Now 4.4 has been branched, should I commit this for RC2, or wait for 4.4.1?
> 
> I've tried to stick with the code style, but it is a little quirky :-)
> 
> 
> This addresses bug 179076.
>     https://bugs.kde.org/show_bug.cgi?id=179076
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepim/messagelist/core/model.cpp 1072229 
> 
> Diff: http://reviewboard.kde.org/r/2521/diff
> 
> 
> Testing
> -------
> 
> Before and after test on my inbox, now sorts correctly.
> 
> 
> Thanks,
> 
> John
> 
>

_______________________________________________
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/