[Kde-pim] Review Request: KMail: Added per-folder aggregation and theme message list settings to folder dialog.

[Thomas McGuire]

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

Ship it!


Thanks, great work!
Please commit.

I have my usual nitpicks, like always (I'm much more critical in reviews than I am with my own code, for some reason).
You can fix those small issues before or after committing, no need for a new review request update.


/trunk/KDE/kdepim/kmail/kmfolderdialog.cpp
<http://reviewboard.kde.org/r/1325/#comment1399>

    This needs context for translators, as "View" can have more meanings.
    See the i18nc call above.



/trunk/KDE/kdepim/kmail/messagelistview/core/aggregationcombobox.h
<http://reviewboard.kde.org/r/1325/#comment1402>

    Constructor should be "explict"
    (Also the other ones)



/trunk/KDE/kdepim/kmail/messagelistview/core/aggregationcombobox.cpp
<http://reviewboard.kde.org/r/1325/#comment1400>

    License can't be LGPL, it has to GPL, as you are using GPL headers.
    (I made the same mistake before as well)



/trunk/KDE/kdepim/kmail/messagelistview/core/aggregationcombobox.cpp
<http://reviewboard.kde.org/r/1325/#comment1401>

    Q_ASSERT would be nicer and an include less.



/trunk/KDE/kdepim/kmail/messagelistview/core/themecombobox.cpp
<http://reviewboard.kde.org/r/1325/#comment1403>

    Can be const I guess (also in the other combobox)


- Thomas


On 2009-08-17 16:04:48, James Bendig wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1325/
> -----------------------------------------------------------
> 
> (Updated 2009-08-17 16:04:48)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Added folder specific aggregation and theme message list settings to the folder properties dialog.
> 
> The way the folder dialog message list default options are applied, functions differently from the existing context menus. When deselecting "Folder Always Uses This ..." in the context menus, the default option is over written with, what was, that folder's private option. This makes sense because of the limited atomic design of context menus.
> 
> The message list settings that I added to the folder dialog keeps all changes local to that dialog's folder. Checking the "Use default ..." checkbox and clicking the Ok button applies the default options as viewed in Settings->Configure KMail->Appearance->Message List.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/kmail/CMakeLists.txt 1012391 
>   /trunk/KDE/kdepim/kmail/configuredialog.cpp 1012391 
>   /trunk/KDE/kdepim/kmail/configuredialog_p.h 1012391 
>   /trunk/KDE/kdepim/kmail/kmfolderdialog.h 1012391 
>   /trunk/KDE/kdepim/kmail/kmfolderdialog.cpp 1012391 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/aggregationcombobox.h PRE-CREATION 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/aggregationcombobox.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/aggregationconfigbutton.h PRE-CREATION 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/aggregationconfigbutton.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/themecombobox.h PRE-CREATION 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/themecombobox.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/themeconfigbutton.h PRE-CREATION 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/themeconfigbutton.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/1325/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> Folder Properties Dialog
>   http://reviewboard.kde.org/r/1325/s/180/
> Folder Properties Dialog, View Tab
>   http://reviewboard.kde.org/r/1325/s/182/
> 
> 
> Thanks,
> 
> James
> 
>

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