2009/2/3 Kevin Ottens <[email protected]> > 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." > > _______________________________________________ > Plasma-devel mailing list > [email protected] > https://mail.kde.org/mailman/listinfo/plasma-devel > > Thank you very much Kevin, your suggestion are really useful to me. I'll commit changes asap :)
P.S. hope you like its new graphical aspect :P cheers -- Alessandro Diaferia
_______________________________________________ Plasma-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/plasma-devel
