Review Request 108667: Let dolphin look at the firstDayOfWeek as set up in the system's locale settings.

Frank Reininghaus frank78ac at googlemail.com
Mon Feb 11 09:28:24 GMT 2013


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


Thanks for the new patch, looks good!

I'm just not sure if the "different weeks in last month" logic should really be removed. I don't use grouping by date much myself, and I don't know if having groups "two weeks ago", etc. really does more harm than good when you're at the beginning of a new month.

What do the others think?

My suggestion would be:

1. Push the rest of the patch to master right now (after considering the inline comments). "Make this month+last month consistent and fix first week day issue" and "reduce confusion by not having too many 'last month' groups" are different issues anyway.

2. If nobody shouts within the next week, it seems that the others agree with your "reduce confusion in the last month" idea, and that nobody minds if you push that too.

Thanks for your work!


dolphin/src/kitemviews/kfileitemmodel.cpp
<http://git.reviewboard.kde.org/r/108667/#comment20460>

    KGlobal is not needed any more, is it?



dolphin/src/kitemviews/kfileitemmodel.cpp
<http://git.reviewboard.kde.org/r/108667/#comment20459>

    I think you don't actually use this header, do you?



dolphin/src/kitemviews/kfileitemmodel.cpp
<http://git.reviewboard.kde.org/r/108667/#comment20461>

    Coding style:
    
    } else {


- Frank Reininghaus


On Feb. 8, 2013, 6:40 p.m., Daniel Kreuter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108667/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2013, 6:40 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> When sorting files by date, dolphin doesn't note the user's system settings. There it's possible to define another first day of week instead of the default (f.e. Sunday instead of Monday).
> This patch fixes this issue.
> 
> 
> This addresses bug 181337.
>     http://bugs.kde.org/show_bug.cgi?id=181337
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodel.cpp a763b3f 
> 
> Diff: http://git.reviewboard.kde.org/r/108667/diff/
> 
> 
> Testing
> -------
> 
> Performed the steps described in comment #3 of the bug report which I won't post here again.
> 
> 
> Thanks,
> 
> Daniel Kreuter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130211/e0ad589d/attachment.htm>


More information about the kfm-devel mailing list