Review Request: Route (un)mounting errors via knotify to the plasma device notifier with fallback to regular notifications

[Olivier Goffart]

Le Wednesday 05 May 2010, Jacopo De Simoi a écrit :
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3895/
> -----------------------------------------------------------
> 
> Review request for kdelibs, Aaron Seigo, Marco Martin, and Olivier Goffart.
> 
> 
> Summary
> -------
> 
> This patch adds a solid device watcher to knotify; the class watches for
> errors and sends them either to a dbus interface (offered by a plasma
> dataengine) or, if the interface is not found, thru regular notifications.


I hate to have to reply by email untill i get my reviewboard password back.

I'm not that in favor to add solid dependence and specific code to knotify. 
But as this is well separated in one different file, this is probably ok if it 
can avoid running another deamon.


In order to work this will require a hardwarenotifications.notifyrc file. does 
this file exist.

In KSolidNotify::KSolidNotify,
you query isServiceRegistered(dbusServiceName), and then you create the 
watcher.
I don't know much of dbus, is there no possibility of a race condition if the 
service is started in between?


You should not mix tab and space.
I guess you should use only spaces for indenting on new code since it is kind 
of the KDE convention nodaways.
(Or you could use tab as knotify uses tab, that are better)
But don't mix.

-- 
Gof



> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/runtime/knotify/CMakeLists.txt 1121868
>   trunk/KDE/kdebase/runtime/knotify/knotify.cpp 1121868
>   trunk/KDE/kdebase/runtime/knotify/ksolidnotify.h PRE-CREATION
>   trunk/KDE/kdebase/runtime/knotify/ksolidnotify.cpp PRE-CREATION
> 
> Diff: http://reviewboard.kde.org/r/3895/diff
> 
> 
> Testing
> -------
> 
> It needs a patch to plasma to work properly, will file that soon to the
> revboard.
> 
> With that patch, it works well.
> 
> 
> Thanks,
> 
> Jacopo