Review Request: big revamp of Device Notifier

[Alessandro Diaferia]

2009/8/22 Giulio Camuffo <giuliocamuffo at gmail.com>

> > 2009/8/21 Alessandro Diaferia <alediaferia at gmail.com>
> >
> > > 2009/8/21 Giulio Camuffo <giuliocamuffo at gmail.com>
> > >
> > >> > On 2009-08-21 17:23:18, Alessandro Diaferia wrote:
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifie
> > >>r.h, line 106
> > >>
> > >> > > <
> > >>
> > >> http://reviewboard.kde.org/r/1370/diff/3/?file=10433#file10433line106
> >
> > >>
> > >> > >     consider deleting white spaces
> > >>
> > >> eh, the problem with white spaces is that they are... white :p they
> > >> escape!
> > >
> > > That's why reviewboard shows them :p
> > >
> > >> > On 2009-08-21 17:23:18, Alessandro Diaferia wrote:
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifie
> > >>r.cpp, line 335
> > >>
> > >> > > <
> > >>
> > >> http://reviewboard.kde.org/r/1370/diff/3/?file=10434#file10434line335
> >
> > >>
> > >> > >     No need for this anymore: i've committed a fixed for the bug
> and
> > >>
> > >> closed it.
> > >>
> > >> whooo! great! thanks
> > >
> > > =) no problem
> > >
> > >> - Giulio
> > >>
> > >>
> > >> -----------------------------------------------------------
> > >> This is an automatically generated e-mail. To reply, visit:
> > >> http://reviewboard.kde.org/r/1370/#review2112
> > >> -----------------------------------------------------------
> > >>
> > >> On 2009-08-21 14:05:09, Giulio Camuffo wrote:
> > >> > -----------------------------------------------------------
> > >> > This is an automatically generated e-mail. To reply, visit:
> > >> > http://reviewboard.kde.org/r/1370/
> > >> > -----------------------------------------------------------
> > >> >
> > >> > (Updated 2009-08-21 14:05:09)
> > >> >
> > >> >
> > >> > Review request for Plasma.
> > >> >
> > >> >
> > >> > Summary
> > >> > -------
> > >> >
> > >> > This is a patch that modifies quite heavily the behaviour of the
> > >> > Device
> > >>
> > >> Notifier.
> > >>
> > >> > It comes from here:
> > >>
> > >> http://kde-look.org/content/show.php/Device+Manager?content=106051
> > >>
> > >> > It can show the not removable devices too, it can mount them
> > >>
> > >> automatically or with a click, since the "eject" button is a "mount"
> > >> button when the volume is umounted. So that guy on the dot will be ok.
> > >>
> > >> > It can hide some items in the same way as Dolphin's places (hide
> item/
> > >>
> > >> show all).
> > >>
> > >> > Finally, it shows the various opening actions under the device
> instead
> > >>
> > >> of calling that xp-ish window.
> > >>
> > >> > Diffs
> > >> > -----
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/CMakeLists.tx
> > >>t 1013960
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/configuration
> > >>page.ui PRE-CREATION
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifie
> > >>r.h 1013960
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifie
> > >>r.cpp 1013960
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespacein
> > >>fodelegate.h 1013960
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespacein
> > >>fodelegate.cpp 1013960
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialo
> > >>g.h 1013960
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialo
> > >>g.cpp 1013960
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.
> > >>h 1013960
> > >>
> > >>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.
> > >>cpp 1013960
> > >>
> > >> > Diff: http://reviewboard.kde.org/r/1370/diff
> > >> >
> > >> >
> > >> > Testing
> > >> > -------
> > >> >
> > >> > I'm using it every day since I released 0.1 on Kde-look. I tried all
> > >> > the
> > >>
> > >> options on my pc and they work. Some people on kde-look posted some
> > >> comments about some problems, but it seems to me they are very
> > >> particular cases, so in my opinion it is quite stable to go in trunk,
> > >> but anyway review it! :)
> > >>
> > >> > Screenshots
> > >> > -----------
> > >> >
> > >> > screen
> > >> >   http://reviewboard.kde.org/r/1370/s/183/
> > >> >
> > >> >
> > >> > Thanks,
> > >> >
> > >> > Giulio
> > >
> > > --
> > > Alessandro Diaferia
> > > KDE Developer
> >
> > Oh one last thing that comes to my mind is that the configuration dialog
> > does not follow GUI usability guidelines. But that shouldn't be a
> problem,
> > there can be a patch for it later :)
> >
> > --
> > Alessandro Diaferia
> > KDE Developer
>
> Yes, I know that I'm not good at all at drawing interfaces, and I don't
> like
> very much the designer. I prefer very much to work underground :)
> Anyway, since I don't have an svn account, if you judge the patch ready,
> feel
> free to commit it.
> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel at kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
>

I've tested the patch and it works correctly. I'd clean indentation a little
before committing. Just waiting until sunday before committing so, if
someone is against the patch, speak now or shut up forever =)

-- 
Alessandro Diaferia
KDE Developer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20090822/d485b13d/attachment-0001.htm