Review Request: Add a title to the account presence menu

[David Edmundson]

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

Ship it!



account-button.h
<http://git.reviewboard.kde.org/r/102023/#comment4270>

    .h files should include the least amount of things possible, it speeds up compiles - especially as this now includes, everything KMenu included.
    
    because you only store a pointer to a KMenu the compiler doesn't need to know how big it is, so we can just forward declare the class with 
    
    class KMenu;
    
    Which you can see we've done for some other classes, such as KLineEdit.
    
    This only applies to storing/passing pointers to objects.
    
    You will then need to #include it in the cpp file.
    
    It's more of a good practice thing, rather than anything actually "wrong".



account-button.cpp
<http://git.reviewboard.kde.org/r/102023/#comment4271>

    You don't really need m_presenceMenu to be a member variable, it could be just local.
    
    If you want access this menu again, you can simply call menu().
    
    Again, you've not anything wrong - just suggesting an alternative that is arguably a teeeeeny bit better and something to think about.


- David


On July 20, 2011, 5:05 p.m., Maxime Corteel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102023/
> -----------------------------------------------------------
> 
> (Updated July 20, 2011, 5:05 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> Added a title to the account presence menu showing the account displayName
> 
> 
> Diffs
> -----
> 
>   account-button.h 8dbf315 
>   account-button.cpp 5c9e135 
> 
> Diff: http://git.reviewboard.kde.org/r/102023/diff
> 
> 
> Testing
> -------
> 
> Changed status
> 
> 
> Screenshots
> -----------
> 
> Menu with title
>   http://git.reviewboard.kde.org/r/102023/s/208/
> 
> 
> Thanks,
> 
> Maxime
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110720/32a92f50/attachment.htm