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

Ship it!


Looks good to me.

- Petri


On 2010-05-14 08:21:03, Michel Lafon-Puyo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/3950/
> -----------------------------------------------------------
> 
> (Updated 2010-05-14 08:21:03)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> As my first work for KDE, I would like to add some power management 
> monitoring features to the system-monitor applet. Monitoring the cpu clock 
> frequency when using the on-demand or the conservative governor could be 
> useful (at least it is for me :)).
> 
> Before going further in this development I tried to fix some little bugs with 
> the applet. Here is the list of the changes this patch introduces:
> - the size of the applets when used in the panel and when no item is 
> monitored was (0,0) but an icon were displayed and it overlapped the other 
> applets (SM::Applet::CheckGeometry())
> - set the preferred height to MINIMUM when no item is monitored 
> (SM::Applet::displayNoAvailableSources()). When all the meters of an applet 
> were removed at once, the size were unchanged and a big icon could be 
> displayed.
> - clear the content of the tooltip when nothing has to be displayed 
> (SM::Applet::toolTipAboutToShow())
> - when one or many items were set "unmonitored", the corresponding widgets 
> were not correctly deleted (SM::Applet::deleteMeters())
> - the removal of the layout and the meters were done on 
> SM::Applet::connectToEngine(), I moved that part in a new method 
> SM::Applet::removeLayout() that can be called more easily. This method is 
> called by the applets on configuration change to achieve a clean update. 
> - the header of the applets is now correctly deleted on form factor change 
> and should not be displayed when the applet is used in the panel
> - when used in the panel, the webview containing the information given by the 
> hardware information applet was displayed under the icon because the webview 
> and the icon were not correctly deleted on form factor changes. 
> - to be consistent with the other applets, the HDD applet has been changed to 
> *not* populate the configuration with the list of the mounted volumes when 
> there is no more item to monitor. Nevertheless, on the first launch (no 
> configuration is present), the behaviour has not changed and the 
> configuration is still populated with the list of the mounted volumes.
> 
> Additionnally, the HDD applet didn't use the SM::Applet to manage its meters. 
> So I replaced the list of SM::Plotter (m_plotters) by a list of 
> QGraphicsWidget (m_meters) and modified HDD to take advantage of the meters 
> management already implemented in the SM::Applet class (in particular the 
> removal of the meters/widgets on configuration change as mentioned above). 
> 
> Note: I was unable to find any bug reports corresponding to the fixes above. 
> Should I create them myself?
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/applet.h 
> 1126529 
>   
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/applet.cpp 
> 1126529 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/cpu.h 
> 1126529 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/cpu.cpp 
> 1126529 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hdd.h 
> 1126529 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hdd.cpp 
> 1126529 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hwinfo.h 
> 1126529 
>   
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hwinfo.cpp 
> 1126529 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/net.h 
> 1126529 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/net.cpp 
> 1126529 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/ram.h 
> 1126529 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/ram.cpp 
> 1126529 
>   
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/temperature.h
>  1126529 
>   
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/temperature.cpp
>  1126529 
> 
> Diff: http://svn.reviewboard.kde.org/r/3950/diff
> 
> 
> Testing
> -------
> 
> Basic testing
> 
> 
> Thanks,
> 
> Michel
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to