Bug 166198 - Review Request: perl-DBIx-ContextualFetch : Add contextual fetches to DBI
Summary: Review Request: perl-DBIx-ContextualFetch : Add contextual fetches to DBI
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Chris Grau
QA Contact: David Lawrence
URL: http://search.cpan.org/dist/DBIx-Cont...
Whiteboard:
Keywords:
Depends On:
Blocks: FE-ACCEPT 166184
TreeView+ depends on / blocked
 
Reported: 2005-08-17 20:45 UTC by Tom "spot" Callaway
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

(edit)
Clone Of:
(edit)
Last Closed: 2005-08-18 04:22:46 UTC


Attachments (Terms of Use)

Description Tom "spot" Callaway 2005-08-17 20:45:11 UTC
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)

Comment 1 Chris Grau 2005-08-18 00:05:51 UTC
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}

Comment 3 Chris Grau 2005-08-18 03:44:41 UTC
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.

Comment 4 Tom "spot" Callaway 2005-08-18 03:46:37 UTC
I knew I missed one! It was nagging me, I thought it was one of the other
packages though. :)

Comment 5 Ralf Corsepius 2005-08-18 07:22:10 UTC
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)

Comment 6 Chris Grau 2005-08-18 15:02:03 UTC
(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.

Comment 7 Ralf Corsepius 2005-08-18 15:40:19 UTC
(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).


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