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."
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
