[KDEREVIEW] DeviceNotifier refactor

[Kevin Ottens]

On Tuesday 3 February 2009 16:57:19 Alessandro Diaferia wrote:
> I've just moved my DeviceNotifier refactoring to kdereview. I feel it is
> much nicer but since i removed tons of code i'd like you to review it and
> tell me if something is wrong.

Warning: I took only a very quick glimpse at it (didn't evne attempt to build 
it, it's really reading code), so it's only shallow reviewing at this point. 
:-)

In my opinion on the code side it looks OK. A couple of warnings though:
 1) I've found a few tabs/space mix. You probably want to remove that and use 
spaces everywhere.
 2) Internally the terminology used is very mount/umount/eject centric. And 
actually that confused me at first, you've to keep in mind that this applet is 
doomed to show devices which aren't storage devices. So for future proofing 
the reading I think you should stick to a more neutral naming, which would 
give for instance for DeviceWidget signals something like:
   void teardownRequested(const QString &udi);
   void activated(const QString &udi);

Hoping that'll help.

Regards.
-- 
Kévin 'ervin' Ottens, http://ervin.ipsquad.net
"Ni le maître sans disciple, Ni le disciple sans maître,
Ne font reculer l'ignorance."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090203/231652f6/attachment-0001.sig