KTp Contact runner

[Martin Klapetek]

Thanks a lot for this, great job!

My review points:

1) We no longer use "org.freedesktop.Telepathy.Client.KDE.TextUi" as the
handler identifier, use "org.freedesktop.Telepathy.Client.KTp.TextUi"
instead, same for filetransfer - replace "KDE" with "KTp" (and watch out
for the general capitalization, we use KTp, you use KTP sometimes) As for
calls (audio/video) - we currently don't really support calls; while it's
cool to have it ready, I'd comment it out for the time being.

2) Thanks for following the kdelibs coding style. There are still some
minor issues left, like with pointer sign, which should be at the variable
side: QObject* parent --> QObject *parent

3) I also agree that keeping the whole model in memory all the time is
probably unwanted. The models are pretty quick to load (try opening contact
list while connected), so it could probably use lazy loading.

4) Avatar icon vs presence icon - I'd suggest to add the presence icon
somewhere else than instead of missing avatar. You could paint is an
overlay, but painting pixmaps over pixmaps could be expensive, so I'd
suggest to place it before the icons for starting chat, audio etc and maybe
visually divide these areas?

Otherwise I'm happy to put it into the 0.4 release.

--
Martin Klapetek | KDE Developer



On Sun, Mar 11, 2012 at 23:22, Dan Vratil <dan at progdan.cz> wrote:

> Hi,
>
> I've made a little Krunner plugin for starting chat with IM contacts and I
> think it would be nice to have it as part of the KTp framework together
> with
> the plasma applets.
>
> For now it's in my scratch repo, but I'd like to move it somewhere public,
> best to the Telepathy project. I'm of course willing to maintain it there,
> there's really not much work since it's really small and simple piece of
> code.
>
> Before any action I'd like to get some feedback from you, because I first
> saw
> Telepathy API 48 hours ago, so there will be some place for improvements :)
>
> Detailed description here:
>
> http://www.progdan.cz/2012/03/kde-telepathy-plugin-for-krunner
>
> and the repo is
>
> git://anongit.kde.org/scratch/dvratil/ktp-contact-runner.git
>
> Regards,
> Dan Vratil
>
> --
> Dan Vratil
> www.progdan.cz | dan at progdan.cz | Jabber: progdan at jabber.cz
> Fingerprint: 76C9 2F08 5D0D 6F9E 5AD4 2BFD 3A85 0307 F506 5B61
> _______________________________________________
> KDE-Telepathy mailing list
> KDE-Telepathy at kde.org
> https://mail.kde.org/mailman/listinfo/kde-telepathy
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120312/1ae04d6f/attachment.html>