KTp Contact runner

[David Edmundson]

2012/3/12 Martin Klapetek <martin.klapetek at gmail.com>:
> Thanks a lot for this, great job!
>
> My review points:
>
Hate to argue [1]


> 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.
>
I thought the same, but it's not an issue, if they don't have the
call-ui, they won't support AV and won't see the "start call" actions.

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

We can optimise later, after we've merged.

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

It actually looks alright as-is.
http://wstaw.org/m/2012/03/12/plasma-desktoptZ1843.png

I think maybe the icon code should maybe just be simplified to
if (hasAvatar) {
 icon = avatar;
} else {
 icon = "that blue playing piece icon thing".
}

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

[1]  That's not true I love to argue.


> --
> 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
>>
>
>
> _______________________________________________
> KDE-Telepathy mailing list
> KDE-Telepathy at kde.org
> https://mail.kde.org/mailman/listinfo/kde-telepathy
>