[Nepomuk] Nepomukcontroller rewritten in QML

Jörg Ehrichs Joerg.Ehrichs at gmx.de
Wed Mar 20 19:55:02 UTC 2013


>
> - Is there a reason nepomukserviceengine.cpp has a different license?
>

No this was simply an error, I'll fix it.

> - You are making a lot of synchronous dbus calls in the dataengine.
> Synchronous dbus calls block all of plasma-desktop until Nepomuk
> replies.
>
> IMHO all of these need to be replaced with the async variants. This
> unfortunately affects a lot of your code. The service job should be
> easy as you don't wait for the results, the service engine is less of
> any easy change. You might want to get a second opinion before
> starting.
>

I was hoping that it could stay this way (less code this way around)
but ideally you are right, this should go async in case one of the
services hangs in such a method (doubt that happens, but
we never know).

But I'll wait for 1-2 more opinions on this first.

> - As far as I can tell all of your QDBusInterface's leak.
> (m_dBusServer, m_serviceFileIndexer, etc) in the ServiceEngine.
> In NepomukServiceJob you take care of it, but you could also add a
> parent parameter which means you won't need to.
>

*whupps* I'll fix this too

> - Plasmoid looks good, only comment I'd suggest is adding a spacing
> between the two buttons. Typically on the desktop this is 4px. I'm
> unsure on plasma rules.
>

The buttons are arranged via PlasmaComponents.ButtonRow so I assume
it was intended to have them close together? Nonetheless I'm fine either way.


More information about the Plasma-devel mailing list