----------------------------------------------------------- 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 > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
