Request for review: perl-Number-Compare

[Ralf Corsepius]

On Fri, 2005-07-01 at 10:32 +0100, Paul Howarth wrote:
> Ralf Corsepius wrote:
> > This package is the first of a lengthy series of perl modules I'd like
> > to see added to FE: perl-Number-Compare.
> > 
> > %description
> > Number::Compare compiles a simple comparison to an anonymous subroutine,
> > which you can call with a value to be tested again.
> > 
> > Files:
> > ftp://packman.iu-bremen.de/fedora/SRPMS/perl-Number-Compare.spec
> > ftp://packman.iu-bremen.de/fedora/SRPMS/perl-Number-Compare-0.01-1.src.rpm
> > 
> > Review (and approval) appreciated, TIA
> 
> Review:
> 
> Good:
> - rpmlint silent
> - package naming meets naming guidelines
> - spec file name matches package name
> - package meets packaging guidelines
> - package has suitable license (Artistic or GPL)
> - spec file written in English and is legible, though I'd personally 
> like to see the header fields line up...
> - source md5sums match upstream
> - builds OK for me on FC4 i386
> - no explicit buildrequires needed
> - no locale data in package
> - no shared libraries in package
> - no relocations
> - directory ownership looks fine
> - no duplicate files
> - permissions fine
> - package has %clean section
> - no compiler optimization macros needed
> - package contains code, not content
> - documentation present and not excessively large
> - nothing in %doc affects runtime
> - no subpackages needed or present
> - no .la files included
> 
> Bad:
> - license text not included in package. This is very common with perl 
> modules and probably not really a blocker. The pod documentation clearly 
> states "This module is free software; you can redistribute it and/or 
> modify it under the same terms as Perl itself."
Right, the license is included into Want.pm.

> - use of macros not quite consistent, e.g. using %{__perl} but not %{__make}
OK, I'll change this.

> Suggestions:
> 
> - I'd expand %description, to explain more the purpose of the module:
> 
>    Number::Compare compiles a simple comparison to an anonymous
>    subroutine, which you can call with a value to be tested again. Now
>    this would be very pointless, if Number::Compare didn't understand
>    magnitudes. The target value may use magnitudes of kilobytes (k, ki),
>    megabytes (m, mi), or gigabytes (g, gi). Those suffixed with an i use
>    the appropriate 2**n version in accordance with the IEC standard:
>    http://physics.nist.gov/cuu/Units/binary.html
> 
> - I'd put %check after %install rather than after %clean, so that builds 
> on rpm versions not supporting %check could still work.
I verified it works on FC3 and FC4 + inside of mock on FC4.

> - I'd base %install on the template in fedora-rpmdevtools rather than 
> the "old-fashioned" way using find. I don't see why a manual 
> INSTALLARCHLIB setting should be necessary for a noarch package either.
Probably true ... and don't actually care about it.

I am using more or less the same spec file for about ca. 30 further modules.

> I'd like to hear further comments on the license text issue here.
I don't see a license issue. 
Want.pm contains:

Copyright (c) 2001-2003, Robin Houston. All Rights Reserved.
This module is free software. It may be used, redistributed
and/or modified under the same terms as Perl itself.

This is the standard perl license as being used by the perl package in FC:
# rpm -q --qf '%{LICENSE}\n' perl
Artistic or GPL

Ralf