Review Request: API to start log/chat/calls

[David Edmundson]

-----------------------------------------------------------
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
> 
>

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