[KDEREVIEW] DeviceNotifier refactor

[Alessandro Diaferia]

2009/2/3 Kevin Ottens <ervin at kde.org>

> 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
> Plasma-devel at kde.org
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20090203/d0315625/attachment.htm