----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107169/#review21364 -----------------------------------------------------------
I like it. George K has a more fancy plan involving desktop files and such, however I think this is a much better solution than what we have now and worth shipping. https://bugs.kde.org/show_bug.cgi?id=295836 KTp/channel-handlers.cpp <http://git.reviewboard.kde.org/r/107169/#comment16622> I think this should be a boolean property passed in, defaulted to true. I think there's some usages of wanting to go straight to the plasmoid, there's a bug report on making in optional. And the contact-list-applet doesn't set this (I think). I think there's certainly a use case for not setting it. TextUI also can look for a hint "org.kde.suppressWindowRaise". Not sure if anyone else would ever want to pass that. Probably not worth doing anything about that one. KTp/channel-handlers.cpp <http://git.reviewboard.kde.org/r/107169/#comment16621> filename -> filePath - David Edmundson On Nov. 2, 2012, 12:24 p.m., Dan Vrátil wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107169/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2012, 12:24 p.m.) > > > Review request for Telepathy. > > > Description > ------- > > I think it's silly to have #define PREFERRED_*_HANDLER everywhere where we > want to handle "start chat/call/..." actions. If we ever want/have to change > the strings, this is the best way to forget about some hidden #define > somewhere and break things. > > Also looking at the ensure*Call, createStreamTube - there are these magic > keywords, like "audio" or "rfb" (really, how is "rfb" related to "desktop > sharing"?) and the useless QDateTime::currentDateTime() being passed > everywhere. > > Finally, because I have problems with remembering KInvocationTool classname, > it also has openLogViewer() method (IIRC David already criticized hardcoding > the ktp-log-viewer name and arguments list everywhere we want to support > logviewer). > > All the methods in the API proposed in this patch only have two arguments: > Tp::AccountPtr and Tp::ContactPtr (and filename in case of filetransfer) - it > looks much cleaner and pretty and it nicely takes care of the magic strings > and #defines that I don't like. > > If you happen to like this proposal, I'll post reviews for porting all > components to the API. > > > Diffs > ----- > > KTp/CMakeLists.txt ae65dc7 > KTp/channel-handlers.h PRE-CREATION > KTp/channel-handlers.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/107169/diff/ > > > Testing > ------- > > > Thanks, > > Dan Vrátil > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
