[Kde-pim] Review Request: show only default address in 'Select recipient' dialog

[Thomas McGuire]

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


As usual, I have some pedantic coding style comments. Especially, I try to avoid iterators, as this leads to hard-to-read "(*it)->" syntax, whereas with foreach, you get "email->", for example.

In general, this looks nice.
However, it crashes as soon as I try it: http://pastebin.com/f644cc6d1
Maybe because that contact doesn't have an email? I can debug further if you can't reproduce this.


/trunk/KDE/kdepim/kmail/recipientspicker.h
<http://reviewboard.kde.org/r/1181/#comment1254>

    please add a name for the parameter.



/trunk/KDE/kdepim/kmail/recipientspicker.h
<http://reviewboard.kde.org/r/1181/#comment1255>

    those methods should be const? and more spaces



/trunk/KDE/kdepim/kmail/recipientspicker.cpp
<http://reviewboard.kde.org/r/1181/#comment1252>

    foreach, if available, would be easier.



/trunk/KDE/kdepim/kmail/recipientspicker.cpp
<http://reviewboard.kde.org/r/1181/#comment1253>

    code duplication: move all those setText() methods into a seperate init() method.



/trunk/KDE/kdepim/kmail/recipientspicker.cpp
<http://reviewboard.kde.org/r/1181/#comment1256>

    using qDeleteAll and clear() seems easier
    (clear() is actually missing here)



/trunk/KDE/kdepim/kmail/recipientspicker.cpp
<http://reviewboard.kde.org/r/1181/#comment1257>

    foreach?


- Thomas


On 2009-07-31 10:21:21, Bruno Bigras wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1181/
> -----------------------------------------------------------
> 
> (Updated 2009-07-31 10:21:21)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Group emails by contact showing the default address in the "Select Recipient" dialog. Based on a commit by Christian Schaarschmidt on the 3.5.5+ branch. (see: https://bugs.kde.org/show_bug.cgi?id=131796#c2 )
> 
> Like the original commit, the "->" column is now the last one to make room for the tree navigation thing. The column is not visible with the default window size, maybe it should be changed.
> 
> 
> This addresses bug 131796.
>     https://bugs.kde.org/show_bug.cgi?id=131796
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/kmail/recipientspicker.h 1004818 
>   /trunk/KDE/kdepim/kmail/recipientspicker.cpp 1004818 
> 
> Diff: http://reviewboard.kde.org/r/1181/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> 
>   http://reviewboard.kde.org/r/1181/s/156/
> 
>   http://reviewboard.kde.org/r/1181/s/157/
> 
> 
> Thanks,
> 
> Bruno
> 
>

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