KEmoticons splitting task

[Kevin Ottens]

Hello,

On Monday 26 August 2013 23:36:13 David Gil Oliva wrote:
> I took the KEmoticons splitting task. I'm studying the code and I have
> found many issues:
> 
> --KEmoticonsProvider should be abstract, since its virtual methods either
> don't do anything or their code is not safe.

Could you be more specific? I don't think I would turn most of them as pure 
virtual. At a glance obvious candidate to be turned pure virtuals are save() 
and createNew()... could be added loadTheme(), addEmoticon() and 
removeEmoticon() with extra care.

> --Many KEmoticons tests don't pass (I haven't actually changed anything
> yet).

Well the autotests seem to pass. There's a few XFAIL but that seems intended.

> And kdelibs-frameworks/staging/kemoticons/tests/main.cpp doesn't
> parse correctly the emoticons. It says 'kemoticonstest(2650)/default
> KEmoticonsPrivate::loadProvider: Invalid plugin factory for
> "emoticonstheme_kde"'. I suspect that the uses of KPluginLoader have to be
> substituted by QPluginLoader in order to work correctly. Could I be right?

Not substituted, but some porting to the new KPlugin* API introduced by sebas 
is indeed needed. There should be porting notes in KDE5PORTING.

> --The API method names could be improved to be clearer. I don't know
> whether to deprecate the old ones or simply substitute them. What's better?

I wouldn't mess around too much with the API for now. What about fixing the 
other issues and then revisiting that?

> --KEmoticonsProvider includes <kio/copyjob.h> and <kio/jobuidelegate.h>. Is
> this OK?

It's OK for a tier 3. Now it seems to be a really minor use, and it's only for 
local use, so if we can somehow spare it that'd be a nice win for this 
framework.

> First I'll try to fix the
> kdelibs-frameworks/staging/kemoticons/tests/main.cpp test to be able to
> know where the problems are when parsing. Then I'll modify the framework to
> be as well written as I can and to have all tests passed. Finally I'll
> split it into tier3 folder. I'll send little patches to RB.

Sounds good. Except for the API changes, keep them for after the rest is done, 
they'll need to be evaluated on a case by case basis.

Regards.
-- 
Kévin Ottens, http://ervin.ipsquad.net

KDAB - proud supporter of KDE, http://www.kdab.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130827/0ffc786d/attachment-0001.sig>