Bug 166183 - Review Request: perl-Class-Accessor-Chained
Review Request: perl-Class-Accessor-Chained
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Chris Grau
David Lawrence
http://search.cpan.org/dist/Class-Acc...
:
Depends On:
Blocks: FE-ACCEPT 166199
  Show dependency treegraph
 
Reported: 2005-08-17 16:16 EDT by Tom "spot" Callaway
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-08-30 21:41:06 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Tom "spot" Callaway 2005-08-17 16:16:30 EDT
Spec Name or Url: 
http://www.auroralinux.org/people/spot/review/Maypole/perl-Class-Accessor-Chained.spec
SRPM Name or Url: 
http://www.auroralinux.org/people/spot/review/Maypole/perl-Class-Accessor-Chained-0.01-2.src.rpm

Description: 

Make chained accessors.

(NOTE: This package is one of the Maypole dependencies)
Comment 1 Chris Grau 2005-08-20 02:08:10 EDT
Good:

- rpmlint clean
- package name okay
- spec file name okay
- license okay (GPL or Artistic)
- license matches upstream
- spec file is am. english (and legible)
- source matches upstream (md5: 9825a1f30a70e55e61bb5660b2bd7365)
- package builds on FC-4/i386
- package builds in mock (FC-3/i386)
- no locales
- no shared libs
- not relocatable
- owns all directories it creates
  (/usr/lib/.../Class owned by perl-Class-Accessor, a prereq)
- file permissions okay
- %clean okay
- consistent use of macros
- code, not content
- no -docs package
- %doc does not affect runtime
- no -devel package

Needswork:

> BuildRequires: perl >= 1:5.6.1

I'm on the fence over whether BR: perl should be included for perl modules. 
It's currently on the ForbiddenItems page, so I'm going to go with not, until
such time as it's removed or an exception is made for perl modules.

- README not in %doc

The README is in POD format.  Maybe pass it through pod2text in %prep?

- no license text in %doc

I let the last few slide without this because I was under the (obviously false)
impression that Perl modules, not being shipped with a license themselves, could
ignore this.  As mentioned in bug 166251, adding

  perldoc -t perlartistic > Artistic
  perldoc -t perlgpl > COPYING

in %prep and then adding them to %doc will do the job without the need to change
anything more than the spec file.

> $ grep -r 'use base' *
> lib/Class/Accessor/Chained/Fast.pm:use base 'Class::Accessor::Fast';
> lib/Class/Accessor/Chained.pm:use base 'Class::Accessor';
[irrelevant results snipped]

=> explicit requires needed for perl(Class::Accessor) and
perl(Class::Accessor::Fast)

Nitpicks:

- make called without %{_smp_mflags}

- %description is just %{summary}

I haven't mentioned it before, but I usually use the first paragraph or two from
the DESCRIPTION section of the main module's POD.  It's usually fitting.
Comment 2 Tom "spot" Callaway 2005-08-28 23:36:50 EDT
Somehow I missed this one. Apologies.

-3 fixes all the items listed above.

New SRPM:
http://www.auroralinux.org/people/spot/review/Maypole/perl-Class-Accessor-Chained-0.01-3.src.rpm

New SPEC:
http://www.auroralinux.org/people/spot/review/Maypole/perl-Class-Accessor-Chained.spec
Comment 3 Chris Grau 2005-08-29 22:00:24 EDT
Two minor items:

- You used %{_smp_mflags} instead of %{?_smp_mflags}.  My fault.  I realize that
I made the typo in my review.

- No changelog entry for -3.

Those can be cleaned up post-import.  Approved.

Note You need to log in before you can comment on or make changes to this bug.