Spec Name or Url: http://www.auroralinux.org/people/spot/review/Maypole/perl-DBIx-ContextualFetch.spec SRPM Name or Url: http://www.auroralinux.org/people/spot/review/Maypole/perl-DBIx-ContextualFetch-1.02-2.src.rpm Description: Add contextual fetches to DBI (NOTE: This package is one of the Maypole dependencies)
Review: - rpmlint clean - package name okay - spec file name okay - license okay - license matches upstream - spec file in am. english (and legible) - source matches upstream - package builds on FC-4 - no locales - no shared libs - not relocatable - all created directories owned - no duplicate %files - file permissions okay - %clean okay - consistent use of macros - code, not content - no -docs - no -devel Needswork: DBI is a requirement in Makefile.PL, but is not listed as a BR. However, the package does build without perl-DBI, but only due to the fact that the tests are skipped without DBD::SQLite2 installed. Since DBD::SQLite2 is available in extras, I think it should be a BR for complete test coverage. perl(DBI) is not automatically picked up as a requires, so it should be explicitly added. I've found this to be a deficiency in RPM that it doesn't automatically pick up modules found in "use base" declarations. Nitpicks: - BuildRequires: perl >= 1:5.6.1 - make called without %{?_smp_mflags}
ALl items fixed in -3: New SRPM: http://www.auroralinux.org/people/spot/review/Maypole/perl-DBIx-ContextualFetch-1.02-3.src.rpm New SPEC: http://www.auroralinux.org/people/spot/review/Maypole/perl-Template-Plugin-Class.spec
The new spec URL is wrong. Everything else looks good. The new spec is missing a changelog entry for -3. Simple enough change, so approved.
I knew I missed one! It was nagging me, I thought it was one of the other packages though. :)
More missing deps: # grep 'use base' usr/lib/perl5/vendor_perl/5.8.6/DBIx/ContextualFetch.pm use base 'DBI'; use base 'DBI::db'; use base 'DBI::st'; => Requires: perl(DBI::db) Requires: perl(DBI::st)
(In reply to comment #5) > More missing deps: > # grep 'use base' usr/lib/perl5/vendor_perl/5.8.6/DBIx/ContextualFetch.pm > use base 'DBI'; > use base 'DBI::db'; > use base 'DBI::st'; > > => > Requires: perl(DBI::db) > Requires: perl(DBI::st) I don't think that's a good idea. Both DBI::db and DBI::st are found inside the DBI.pm file (hidden to prevent PAUSE indexing), but they're not explicitly provided by perl-DBI. So then we'd end up with dependencies that could not be resolved. I suppose that perl-DBI could be changed to explicitly provide these, but in the meantime, perl(DBI) is enough to resolve dependencies.
(In reply to comment #6) > (In reply to comment #5) > > More missing deps: > I suppose that perl-DBI could be changed to explicitly provide these, but > in the meantime, perl(DBI) is enough to resolve dependencies. Well, IMO, this is a case of 2 "falses" (Missing provides in perl-DBI, missing deps in perl-DBIx-ContextualFetch) giving one "right" (Correct operation).