[Bug 513797] Review Request: gnome-applet-cpufire - GNOME panel applet showing the CPU load as a fire

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=513797


Christoph Wickert <fedora at christoph-wickert.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|needinfo?(fedora at christoph- |
                   |wickert.de)                 |




--- Comment #6 from Christoph Wickert <fedora at christoph-wickert.de>  2009-07-30 20:07:36 EDT ---
Thanks for this detailed review. I really like people reviewing carefully and
I'm glad I sponsored you.

(In reply to comment #5)
> In summary:
> - rpmlint warning to be fixed

done

> - license does not seem to match

The package includes a copy of GPLv2, but it's unclear wether is "GPLv2 only"
or GPLv2 "or any later version". The headers of the source contain no license
block ether, so I mailed the author and his reply was that it's "clearly
GPLv2+". Quote: "Das steht aber klar im COPYRIGHT: GPLv2+"
In line 298/299 of COPYING also "or any later version" is mentioned, so GPLv2+
definitely is correct.

> - comment out libgnomeui-devel in BuildRequires including comment as to why

Why should I comment out libgnomeui? It's needed, cpufire_applet.c has
...
#include <libgnomeui/libgnomeui.h>
...

> - either add comment to, or remove BR GConf2 commented out

During configure you'll see:
...
Using config source xml:merged:/etc/gconf/gconf.xml.defaults for schema
installation
Using $(sysconfdir)/gconf/schemas as install directory for schema files
...

Only GConf2 is needed, not GConf2-devel. As GConf is already pulled in by
several other BuildRequires I removed it.

> - explain why you need gnome-panel-devel >= 2.6

...
#include <panel-applet.h>
#include <panel-applet-gconf.h>
...
$ rpm -qf /usr/include/panel-2.0/panel-applet.h
/usr/include/panel-2.0/panel-applet-gconf.h 
gnome-panel-devel-2.26.3-1.fc11.x86_64
gnome-panel-devel-2.26.3-1.fc11.x86_64

> - own %{_datadir}/omf/cpufire/

fixed

> - optionally: include the empty files MAINTAINERS, NEWS and TODO  

I agree with you that including these 0kb files is easier, but common practice
in Fedora is not to include them, otherwise rpmlint will complain again. I will
add them as soon as they have content.

New files:
Spec: http://cwickert.fedorapeople.org/review/gnome-applet-cpufire.spec
SRPM:
http://cwickert.fedorapeople.org/review/gnome-applet-cpufire-1.6-2.fc11.src.rpm

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.