[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: [Nepomuk] Nepomukcontroller rewritten in QML
From:       David Edmundson <david () davidedmundson ! co ! uk>
Date:       2013-03-19 19:39:58
Message-ID: CAGeFrHAvLSahLvgR6sR5JbDZ8fX5qRqDHi-zzmSqEhPi4UqzMA () mail ! gmail ! com
[Download RAW message or body]

On Tue, Mar 19, 2013 at 6:34 PM, J=F6rg Ehrichs <Joerg.Ehrichs@gmx.de> wrot=
e:
> 2013/3/19 Ignacio Serantes <kde@aynoa.net>:
>> Hi,
>>
>> Installed and is an amazing improvement. Only for contribute with minor
>> thing:
>>
>> A method to call a manual backup or a restore.
>> A start/stop/restart nepomuk method.
>>
>
> Start/Stop for any service is available in the Plasma::DataEngine you can
> check with the plasmaengineexplorer but was not exposed to the user on
> purpose as discussed with Vishesh.
>
> But a frontend for easy backup/restore sounds like a good idea

I would recommend getting it merged as-is before you start looking at
extra features.

Looks like a really good start, I've got a few comments:

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

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

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

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


> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]