Review Request: Make display name and icon configurable from accounts kcm

[David Edmundson]

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


Bunch of little fixes to do.

Other than that, all good. 

Though as normal we should monitor feedback, and adjust appropriately if there are any comments (just like the presence combo).


src/accounts-list-delegate.cpp
<http://git.reviewboard.kde.org/r/104323/#comment9174>

    not really a fan of magic numbers.
    
    It's better to have names, so they can be changed (I've highlighted the other time we use this) in one place without exploding.



src/accounts-list-delegate.cpp
<http://git.reviewboard.kde.org/r/104323/#comment9180>

    won't -> will not.
    
    (it's more formal to avoid contractions)



src/accounts-list-delegate.cpp
<http://git.reviewboard.kde.org/r/104323/#comment9177>

    Do these get leaked? 
    
    documentation on KWidgetItemDelegate is a bit sparse.
    
    If you have checked, add a comment so we don't all pester you constantly :)



src/accounts-list-delegate.cpp
<http://git.reviewboard.kde.org/r/104323/#comment9182>

    QIcon? (you'll get Krazy comments on not using KIcon)
    
    Also I /think/ that
    
    QIcon accountIcon(index.data(....));
    is faster than
    QIcon accountIcon = index.data();
    
    which has an extra copy. Though tbh, I don't really care.



src/accounts-list-delegate.cpp
<http://git.reviewboard.kde.org/r/104323/#comment9183>

    magic numbers are naughty.
    But well done on the padding! (you know how much I love them :) )



src/change-icon-button.h
<http://git.reviewboard.kde.org/r/104323/#comment9184>

    const &



src/change-icon-button.cpp
<http://git.reviewboard.kde.org/r/104323/#comment9156>

    const &



src/edit-display-name-button.h
<http://git.reviewboard.kde.org/r/104323/#comment9185>

    const &



src/edit-display-name-button.cpp
<http://git.reviewboard.kde.org/r/104323/#comment9186>

    const &



src/edit-display-name-button.cpp
<http://git.reviewboard.kde.org/r/104323/#comment9187>

    I personally really prefer people using UI files. 
    
    Not worth changing now though.


- David Edmundson


On March 18, 2012, 2:10 a.m., Daniele Elmo Domenichelli wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104323/
> -----------------------------------------------------------
> 
> (Updated March 18, 2012, 2:10 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> There are a few rendering bugs in KWidgetItemDelegate, but the ugliest is fixed in kde 4.8.1, one is fixed in kdelibs KDE/4.8 branch and another one in in review
> 
> 
> Diffs
> -----
> 
>   src/change-icon-button.h PRE-CREATION 
>   src/change-icon-button.cpp PRE-CREATION 
>   src/edit-display-name-button.h PRE-CREATION 
>   src/edit-display-name-button.cpp PRE-CREATION 
>   src/accounts-list-delegate.cpp e818582ca3bd9b96079b1a0c9f10997b7649caf1 
>   src/CMakeLists.txt 8fb139a673326be47b08c695c13c78d7534d1afb 
>   src/accounts-list-delegate.h 2c8b638ce244c261e9a8a462e3dca93ebd88948a 
> 
> Diff: http://git.reviewboard.kde.org/r/104323/diff/
> 
> 
> Testing
> -------
> 
> Works
> 
> 
> Screenshots
> -----------
> 
> 
>   http://git.reviewboard.kde.org/r/104323/s/478/
> 
>   http://git.reviewboard.kde.org/r/104323/s/479/
> 
> 
> Thanks,
> 
> Daniele Elmo Domenichelli
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120318/0fcffbb9/attachment-0001.html>