I've merged touchpad-kcm frameworks branch into master and I'll include it in the Plasma 5.2 Beta release.
I've also made Rajeesh the default assignee for new bugs on bugzilla. The blocker to releasing it is that it overlaps with ktouchpadenabler somewhat but it doesn't set the default keyboard accelarator right, I've filed a bug and marked it release_blocker and I don't expect to make a stable release until it gets fixed https://bugs.kde.org/show_bug.cgi?id=342629 Jonathan On 4 January 2015 at 12:52, Rajeesh K Nambiar <rajeeshknamb...@gmail.com> wrote: > On Sat, Jan 3, 2015 at 7:56 PM, David Edmundson > <da...@davidedmundson.co.uk> wrote: > > > > > > On Sat, Jan 3, 2015 at 7:07 PM, Rajeesh K Nambiar > > <rajeeshknamb...@gmail.com> wrote: > >> > >> On Thu, Dec 11, 2014 at 4:30 PM, David Edmundson > >> <da...@davidedmundson.co.uk> wrote: > >> > 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. > >> > >> Thanks for the review. I cannot answer many of the comments, but some > >> queries below: > >> > >> > > >> > kded.cpp > >> > The touchpad backend leaks? > >> > >> TouchpadBackend::implementation has static variable, would this still > >> cause leak? (especially if there are multiple backends...) > >> > >> > > >> > 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 couldn't find a way to work around this as there's no async > >> alternative to isServiceRegistered. Could Alexander help? > > > > > > Ah, it's a bit obscure. > > > > QDBusPendingCall async = > > > QDBusConnection::sessionBus().interface()->asyncCall(QLatin1String("ListNames")); > > QDBusPendingCallWatcher *callWatcher = new > > QDBusPendingCallWatcher(async, this); > > connect(callWatcher, SIGNAL(finished(QDBusPendingCallWatcher*)), > > this, > SLOT(serviceNameFetchFinished(QDBusPendingCallWatcher*))); > > > > > > void serviceNameFetchFinished(QDBusPendingCallWatcher *callWatcher) > > { > > QDBusPendingReply<QStringList> reply = *callWatcher; > > callWatcher->deleteLater(); > > > > if (reply.isError()) { > > qDebug() << "omg wtf bbq"; > > return; > > } > > > > QStringList allServices = reply.value(); > > if (allService.contains("MYSERVICE")) { > > //do stuff > > } > > } > > > > Thank you. I have made some changes and tested it - doesn't seem to > break things. Opened a review request here: > https://git.reviewboard.kde.org/r/121825/ . > > > > >> > >> > > >> > 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 > >> > >> Top level Messages.sh does put .{h,cpp} file translations to > >> kcm_touchpad.pot and {qml,js} file translations to > >> plasma_applet_touchpad.pot - could you be more specific if this needs > >> to change? > >> > >> > 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. > >> > >> But this makes sense to leave it as it is - as users would want to > >> 'disable' tap+scrolling alone. > >> > >> > > >> > 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. > >> > >> > > -- > Rajeesh > _______________________________________________ > 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
