Review Request 117570: Added join message capability

[Daniele E. Domenichelli]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117570/#review55783
-----------------------------------------------------------



config/appearance-config-tab.cpp
<https://git.reviewboard.kde.org/r/117570/#comment38810>

    Perhaps it is a stupid comment, but the order of the events does not make sense.
    
    I'd put the "leave" event after this one as an history event (AdiumThemeStatusInfo(false), no changes needed), followed by this "join" event as not-history (AdiumThemeStatusInfo(false))



config/appearance-config.ui
<https://git.reviewboard.kde.org/r/117570/#comment38807>

    Perhaps it should be join/part? If you change this perhaps also the names of the methods/variables should be updated



lib/chat-widget.h
<https://git.reviewboard.kde.org/r/117570/#comment38811>

    Since you are changing this, perhaps you could make it with the same signature as  Tp::Channel::groupMembersChanged http://telepathy.freedesktop.org/doc/telepathy-qt/a00106.html#a9a19634088f78c3fb81fd166df72739e , it will be easier in the future if we want to support the part/quit/kick/ban messages. (Also perhaps using groupLocalPendingMembersAdded and groupRemotePendingMembersAdded it could be possible to show contacts invited that did not accept the invitation yet? Anyway that's for some future patch)


Besides these comments, I tested it and it works as expected, so it's a ship-it from me

- Daniele E. Domenichelli


On April 14, 2014, 8:02 p.m., Daniel Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117570/
> -----------------------------------------------------------
> 
> (Updated April 14, 2014, 8:02 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-text-ui
> 
> 
> Description
> -------
> 
> Added join message capability. Configuration of displaying it is tied in with the option for showing people leaving.
> 
> 
> Diffs
> -----
> 
>   config/appearance-config-tab.h 9582660 
>   config/appearance-config-tab.cpp f8314d8 
>   config/appearance-config.ui 6440cba 
>   lib/adium-theme-view.h 5eaab55 
>   lib/adium-theme-view.cpp e0ad0b1 
>   lib/chat-widget.h 5428104 
>   lib/chat-widget.cpp 3929e45 
> 
> Diff: https://git.reviewboard.kde.org/r/117570/diff/
> 
> 
> Testing
> -------
> 
> Joined and left with several accounts into a chat room.
> 
> 
> Thanks,
> 
> Daniel Cohen
> 
>

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