Review Request: The Delegate Channels Patch

[Lasath Fernando]

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


Overall, looks impressive. 

But I still don't get why you moved the handler directly into ConversationsModel


plasmoid/declarative-plugin/conversation.h
<http://git.reviewboard.kde.org/r/103908/#comment8576>

    I read recently that according to KDE style guide, you're supposed to have local #includes before system ones.
    
    i.e 
    #include "our-stuff.h"
    
    #include <Qt/Stuff>
    
    #include <KDE/Stuff>



plasmoid/declarative-plugin/conversations-model.cpp
<http://git.reviewboard.kde.org/r/103908/#comment8579>

    Why did you remove parent? 



plasmoid/declarative-plugin/telepathy-text-observer.cpp
<http://git.reviewboard.kde.org/r/103908/#comment8584>

    This isn't parented nor deleted at the end.



plasmoid/declarative-plugin/telepathy-text-observer.cpp
<http://git.reviewboard.kde.org/r/103908/#comment8585>

    I kind of thought up of this in a hurry. Should we try and come up with a better handler name?



plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ChatWidget.qml
<http://git.reviewboard.kde.org/r/103908/#comment8580>

    trailing whitespace



plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ChatWidget.qml
<http://git.reviewboard.kde.org/r/103908/#comment8581>

    trailing whitespace. wow this thing makes it really easy to notice



plasmoid/org.kde.ktp-chatplasmoid/contents/ui/main.qml
<http://git.reviewboard.kde.org/r/103908/#comment8582>

    m0ar trailing whitespace...



plasmoid/org.kde.ktp-chatplasmoid/contents/ui/main.qml
<http://git.reviewboard.kde.org/r/103908/#comment8583>

    again. how come your IDE doesn't automatically remove them?


- Lasath Fernando


On Feb. 9, 2012, 11:08 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103908/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2012, 11:08 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> I'm afraid this patch is a bit huge, but that's just because I've been working hard :-D
> 
> In the plasmoid:
> Convert plasmoid to a handler from an observer
> Support delegate channels (via a nasty hack currently)
> Add popout button to delegate to main text UI
> Merge the conversationsModel and handler into one class
> Fix exposing a load of items to QML for no reason.
> Some additional tidying. (and some additional messying :-) )
> 
> In the textUI:
> Always raise the window in the main Text UI handleChannel unless hint explicitly says otherwise.
> Additionally fixes a bug in which reconnecting an account could cause the tabs to 'randomly' switch.
> 
> Requires a one line patch in the contact list:
> 
> -    hints.setHint("org.kde.telepathy","forceRaiseWindow", QVariant(true));
> 
> +    hints.setHint("org.freedesktop.Telepathy.ChannelRequest","DelegateToPreferredHandler", QVariant(true)); 
> 
> I'll do delegateChannels properly in TpQt at some point, but I need to know what would happen and we can get this work going in parallel. Also means I have a nice way to test TpQt.
> 
> 
> Diffs
> -----
> 
>   app/chat-tab.cpp a39526319d90d4c5ddb8e252929f6a0ba3c58640 
>   app/telepathy-chat-ui.cpp 91ef0e323956c3e376b15843d2f531a277f10a95 
>   plasmoid/declarative-plugin/channel-delegator.h PRE-CREATION 
>   plasmoid/declarative-plugin/conversation.h 78fd38dffa7e5e0df230c55440519c05f47f8bf4 
>   plasmoid/declarative-plugin/conversation.cpp c2f707a10e3f7715ac1e9a2af3c6d857f1f28f38 
>   plasmoid/declarative-plugin/conversations-model.h a4c0f0382f44980445c53cb49df5b367a2ab6146 
>   plasmoid/declarative-plugin/conversations-model.cpp 3967bffa9cad05e33e63a6097ef856ed41eeaa50 
>   plasmoid/declarative-plugin/messages-model.h fe47a0092deb632e795f644c95224bff599553ed 
>   plasmoid/declarative-plugin/messages-model.cpp 6cc7c90eae3f1a8ad34e661c2983488ec0f273ac 
>   plasmoid/declarative-plugin/qml-plugins.cpp 44effaebba2ad04a5a4ea859350de58d7c0d34e5 
>   plasmoid/declarative-plugin/telepathy-text-observer.h 6658ee57db72abea2c3ae53407467d64fdd47273 
>   plasmoid/declarative-plugin/telepathy-text-observer.cpp 58cf736834fe194a9b9088b4086ad3dcabbe95df 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ChatWidget.qml 07b67ecfb6a731be758b3ed3624b1be56b1e3411 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/main.qml 0af180b35cf60074b33b5f4a53547b21e5c1d474 
> 
> Diff: http://git.reviewboard.kde.org/r/103908/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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