Review Request: big revamp of Device Notifier

[Jacopo De Simoi]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1370/#review2213
-----------------------------------------------------------


I previously overlooked this; the mounting procedure should belong to notifierdialog, as the unmounting. Please remove all the slot/signal stuff. It's not necessary and makes the code more complicated. 


/trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp
<http://reviewboard.kde.org/r/1370/#comment1511>

    I do not see why this method belongs here. 
    Unmounting is done internally by the notifierdialog, so mounting should be taken care of similarly. 
    Please move this piece of code where it belongs (see the other note)



/trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.cpp
<http://reviewboard.kde.org/r/1370/#comment1512>

    Please move the mounting method here, there's no need for the signal/slot dance.
    Moreover this makes it possible to trigger the error dialog in case something goes wrong (see right above)


Thanks

- Jacopo


On 2009-08-21 19:26:43, Giulio Camuffo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1370/
> -----------------------------------------------------------
> 
> (Updated 2009-08-21 19:26:43)
> 
> 
> 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/devicespaceinfodelegate.cpp 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.h 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.h 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/configurationpage.ui PRE-CREATION 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/CMakeLists.txt 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.h 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.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
> 
>