'Re: Review Request: bug fixes for the system-monitor applet'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request: bug fixes for the system-monitor applet
From:       "Aaron Seigo" <aseigo () kde ! org>
Date:       2010-05-11 16:59:57
Message-ID: 20100511165957.12975.23315 () localhost
[Download RAW message or body]

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

"Note: I was unable to find any bug reports corresponding to the fixes above. Should \
I create them myself?"

you don't need to create bug reports, no :)

- Aaron

On 2010-05-11 16:25:30, Michel Lafon-Puyo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3950/
> -----------------------------------------------------------
> 
> (Updated 2010-05-11 16:25:30)
> 
> 
> 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 1125154 \
>                 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/applet.cpp \
>                 1125154 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/cpu.h 1125154 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/cpu.cpp 1125154 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hdd.h 1125154 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hdd.cpp 1125154 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hwinfo.h 1125154 \
>                 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hwinfo.cpp \
>                 1125154 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/net.cpp 1125154 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/ram.cpp 1125154 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/temperature.cpp \
> 1125154  
> Diff: http://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

[prev in list] [next in list] [prev in thread] [next in thread] 