Review Request: API to start log/chat/calls
David Edmundson
david at davidedmundson.co.uk
Tue Nov 13 02:55:35 UTC 2012
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107169/#review21910
-----------------------------------------------------------
Ship it!
Ship It!
- David Edmundson
On Nov. 12, 2012, 9:45 a.m., Dan Vrátil wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107169/
> -----------------------------------------------------------
>
> (Updated Nov. 12, 2012, 9:45 a.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 0fd12f0
> KTp/actions.h PRE-CREATION
> KTp/actions.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/20121113/76fae214/attachment.html>
More information about the KDE-Telepathy
mailing list