Review Request: Add buttons to start desktop sharing from the contact list

David Edmundson kde at davidedmundson.co.uk
Tue Sep 20 09:55:28 UTC 2011



> On Sept. 20, 2011, 9:06 a.m., Martin Klapetek wrote:
> > service-availability-checker.cpp, line 41
> > <http://git.reviewboard.kde.org/r/102668/diff/1/?file=36504#file36504line41>
> >
> >     I'm very likely missing something, but where does this pointer gets deleted?

It has a parent item, so when ServiceAvailability gets deleted


> On Sept. 20, 2011, 9:06 a.m., Martin Klapetek wrote:
> > service-availability-checker.cpp, line 83
> > <http://git.reviewboard.kde.org/r/102668/diff/1/?file=36504#file36504line83>
> >
> >     Why is this a pointer?

This part took me several times to read. It's a pointer to one of the two instance variable booleans, d->serviceAvailable or d->serviceActivatable (or 0).

Tbh, this code could possibly do with being clearer rather than clever.


- David


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


On Sept. 19, 2011, 6:07 p.m., George Kiagiadakis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102668/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2011, 6:07 p.m.)
> 
> 
> Review request for Telepathy and Martin Klapetek.
> 
> 
> Summary
> -------
> 
> This patch adds a button and a context menu action to every contact in the contact list that allows users to start desktop sharing with the selected contact. The patch also includes a class that checks for the availability of krfb on the system, as there is no way of determining that via telepathy.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 7c463ff50946c4e9b307ed98e41dbf6b4f84a5be 
>   contact-overlays.h 43bcf237397fc2770d834cb29275da17456cf4e2 
>   contact-overlays.cpp 026e3d34b7ceb92e3415a558d15c73293648a2e6 
>   main-widget.h 6843af5af4b0b7d32eccb835793f0a0caec15377 
>   main-widget.cpp 98abf2968bc645f4c78e99806ca264b17e5ccbfc 
>   models/accounts-model.h 40b915b78637dc4d4e2b3d85aa559d4f0b465345 
>   models/accounts-model.cpp 75d224ff2e332bbd3f96293854396d1a28e23ff9 
>   models/contact-model-item.h cc2a59f713c9de59ecfe95558c7dbed5acf82570 
>   models/contact-model-item.cpp c32b4ffe6502af074bddd6687a623c2d5673508d 
>   service-availability-checker.h PRE-CREATION 
>   service-availability-checker.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102668/diff
> 
> 
> Testing
> -------
> 
> Successfully started a desktop sharing session between my laptop and my desktop with krfb. However the client side failed to show the desktop after approving the channel (this is either a bug in krfb - most likely - or something in gabble; I'll inspect it)
> 
> 
> Thanks,
> 
> George
> 
>

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


More information about the KDE-Telepathy mailing list