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 Edwin ten Brink <fedora@xxxxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(fedora@christoph- | |wickert.de) --- Comment #7 from Edwin ten Brink <fedora@xxxxxxxxxxxxxxxxxxx> 2009-07-31 03:31:22 EDT --- (In reply to comment #6) > Thanks for this detailed review. I really like people reviewing carefully and > I'm glad I sponsored you. You're welcome. I appreciate your sponsorship. > (In reply to comment #5) > > In summary: > > - rpmlint warning to be fixed > > done OK: rpmlint is quiet now. > > - 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. Line 298/299 is not part of the license, but merely an example of how to include GPLv2(+) in your programs. As most people choose to literally take the text (which is not a bad idea), this text can usually be found in headers and/or source files. However, the author did not do so. Stricly legally, therefore only GPLv2 is applicable, IMO. It would be better if the author would include the text and actions as mentioned in GPLv2 (lines 282-311), perhaps you could communicate that to upstream. OK: If the author indeed confirmed that GPLv2+ is applicable, then you can leave it as it is (but please contact the author). > > - 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> > ... NOT OK: I agree that you need libgnomeui-devel, but libgnomeui-devel gets pulled in by gnome-panel-devel. Again, as per http://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires, it should be removed. We implicitly agreed on this during the review of gnome-applet-bubblemon (bug 497525): http://cvs.fedoraproject.org/viewvc/rpms/gnome-applet-bubblemon/devel/gnome-applet-bubblemon.spec?revision=1.1&view=markup > > - 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. OK. > > - 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 NOT OK: I'm sorry the comment appears to be unclear. I know you depend on gnome-panel-devel, but why did you state version >= 2.6. This requirement on the version is not documented in the tarball, and AFAICS, is not checked by the configure script. If you indeed require >= 2.6, then this version is also 'very old'. Fedora 7 included 2.18.3-1. As per http://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires, you should drop te version requirement. > > - own %{_datadir}/omf/cpufire/ > > fixed OK. > > - 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. OK. Open issues: - superfluous dependency on libgnomeui-devel in BuildRequires - versioned dependency on gnome-panel-devel >= 2.6 in BuildRequires - contact the upstream author to see if he would include the text and actions as mentioned in GPLv2 (lines 282-311) in the next release -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review