Review Request: Add "join chat room" action in contact list

[David Edmundson]

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


Not a bad go. Few comments.


dialogs/join-chat-room-dialog.cpp
<http://git.reviewboard.kde.org/r/101751/#comment3378>

    Why do this?
    
    You can replace those 4 lines with
    
    ui->setupUi(this);



dialogs/join-chat-room-dialog.cpp
<http://git.reviewboard.kde.org/r/101751/#comment3380>

    This is a bit weak, it's possible to have two accounts with the same display name.
    
    Ideally we should check the account paths.
    
    Obviously you can't then compare against the text, so to get round this, you can add extra item data to each entry of the combobox, and check against that.
    
    See also add-contact-dialog, that does something similar (though that stores Account-Model-Items* in the comboBox)
    That uses the model to get an account list. 
    
    



main-widget.cpp
<http://git.reviewboard.kde.org/r/101751/#comment3379>

    that's the weirdest account of "account" I've ever seen.
    
    Also remove this debug line, or at least make it use kDebug


- David


On June 24, 2011, 6:54 p.m., Francesco Nwokeka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101751/
> -----------------------------------------------------------
> 
> (Updated June 24, 2011, 6:54 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> Adds an action under the "wrench" button to join a chat group. Only online accounts with this capability are listed.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3dd8cc0 
>   dialogs/join-chat-room-dialog.h PRE-CREATION 
>   dialogs/join-chat-room-dialog.cpp PRE-CREATION 
>   main-widget.h 5625778 
>   main-widget.cpp f19cbee 
> 
> Diff: http://git.reviewboard.kde.org/r/101751/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Francesco
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110624/66c16975/attachment-0001.htm