--===============0797815241== Content-Type: multipart/alternative; boundary="===============5789368726618575344==" --===============5789368726618575344== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- 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 monit= oring features to the system-monitor applet. Monitoring the cpu clock frequ= ency 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 moni= tored was (0,0) but an icon were displayed and it overlapped the other appl= ets (SM::Applet::CheckGeometry()) > - set the preferred height to MINIMUM when no item is monitored (SM::Appl= et::displayNoAvailableSources()). When all the meters of an applet were rem= oved 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 widget= s were not correctly deleted (SM::Applet::deleteMeters()) > - the removal of the layout and the meters were done on SM::Applet::conne= ctToEngine(), 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 con= figuration change to achieve a clean update. = > - the header of the applets is now correctly deleted on form factor chang= e 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 w= ebview and the icon were not correctly deleted on form factor changes. = > - to be consistent with the other applets, the HDD applet has been change= d 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 (n= o configuration is present), the behaviour has not changed and the configur= ation is still populated with the list of the mounted volumes. > = > Additionnally, the HDD applet didn't use the SM::Applet to manage its met= ers. So I replaced the list of SM::Plotter (m_plotters) by a list of QGraph= icsWidget (m_meters) and modified HDD to take advantage of the meters manag= ement already implemented in the SM::Applet class (in particular the remova= l of the meters/widgets on configuration change as mentioned above). = > = > Note: I was unable to find any bug reports corresponding to the fixes abo= ve. Should I create them myself? > = > = > Diffs > ----- > = > /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/appl= et.h 1126529 = > /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/appl= et.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/hwin= fo.h 1126529 = > /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hwin= fo.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/temp= erature.h 1126529 = > /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/temp= erature.cpp 1126529 = > = > Diff: http://svn.reviewboard.kde.org/r/3950/diff > = > = > Testing > ------- > = > Basic testing > = > = > Thanks, > = > Michel > = > --===============5789368726618575344== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://svn.reviewb= oard.kde.org/r/3950/

Ship it!

Looks good=
 to me.

- Petri


On May 14th, 2010, 8:21 a.m., Michel Lafon-Puyo wrote:

Review request for Plasma.
By Michel Lafon-Puyo.

Updated 2010-05-14 08:21:03

Descripti= on 

As my first work for KDE, I would like to add some power man=
agement monitoring features to the system-monitor applet. Monitoring the cp=
u clock frequency when using the on-demand or the conservative governor cou=
ld be useful (at least it is for me :)).

Before going further in this development I tried to fix some little bugs wi=
th 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 monito=
red was (0,0) but an icon were displayed and it overlapped the other applet=
s (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 remov=
ed 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::Ap=
plet::toolTipAboutToShow())
- when one or many items were set "unmonitored", the correspondin=
g widgets were not correctly deleted (SM::Applet::deleteMeters())
- the removal of the layout and the meters were done on SM::Applet::connect=
ToEngine(), I moved that part in a new method SM::Applet::removeLayout() th=
at can be called more easily. This method is called by the applets on confi=
guration 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 t=
he hardware information applet was displayed under the icon because the web=
view 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 wh=
en there is no more item to monitor. Nevertheless, on the first launch (no =
configuration is present), the behaviour has not changed and the configurat=
ion is still populated with the list of the mounted volumes.

Additionnally, the HDD applet didn't use the SM::Applet to manage its m=
eters. So I replaced the list of SM::Plotter (m_plotters) by a list of QGra=
phicsWidget (m_meters) and modified HDD to take advantage of the meters man=
agement already implemented in the SM::Applet class (in particular the remo=
val 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?

Testing <= /h1> 
Basic testing

Diffs=

  • /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/app= let.h (1126529)
  • /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/app= let.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/hwi= nfo.h (1126529)
  • /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hwi= nfo.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/tem= perature.h (1126529)
  • /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/tem= perature.cpp (1126529)

View Diff
--===============5789368726618575344==-- --===============0797815241== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel --===============0797815241==--