kcm-touchpad in plasma 5.2?

[David Edmundson]

In general this looks OK, there's some useful features and I can see myself
using this. I'd very much like it to become part of Plasma.

I gave it a review and made some notes below.

kded.cpp
The touchpad backend leaks?

There are blocking calls in the constructor using isServiceRegistered
combined with the dataengine querying this kded module in a blocking way in
init we have a strong potential to deadlock the two applications
For KDED modules we have to be a lot more strict to never block as others
will be querying us.

I don't understand why we're watching for plasmashell/kglobalaccel anyway.
Could you explain the rationale here.

applet:
The applet is completely not ported.

Applet translations need to go into a differnet .po file with a specific
name
Copy a Messages.sh from one of the other plasmoids

KCM:
reverse scrolling doesn't seem to work
"disabled taps and scrolling only" -- The HIG says to avoid negated
checkbox descriptions.

The translation_domain doesn't seem to be set, so kded/kcm won't load
anything

touchpad backend.h:
This class shouldn't be instantiated directly, don't make the constructor
public.

The X backend:
This looked scary so I didn't review it at all.
​
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20141211/2051b6ae/attachment-0001.html>