Bug 188261 - Review Request: perl-Finance-Quote
Review Request: perl-Finance-Quote
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul Howarth
Fedora Package Reviews List
: Reopened
: 161410 (view as bug list)
Depends On: 188293
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-04-07 10:52 EDT by Bill Nottingham
Modified: 2014-03-16 22:59 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-06-12 16:12:13 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Bill Nottingham 2006-04-07 10:52:18 EDT
Spec Name or Url: http://people.redhat.com/notting/p-f-q/perl-Finance-Quote.spec
SRPM Name or Url: http://people.redhat.com/notting/p-f-q/perl-Finance-Quote-1.11-1.src.rpm
Description: A perl module that grabs quotes from various sources.
Comment 1 Bill Nottingham 2006-04-07 10:53:40 EDT
*** Bug 161410 has been marked as a duplicate of this bug. ***
Comment 2 Paul Howarth 2006-04-07 11:10:37 EDT
Please submit a package for perl(HTML::TableExtract) too, since it is a
dependency of this package.

The "BuildRequires: perl >= 1:5.6.1" is redundant for Fedora Extras

The "|| :" after "%check" shouldn't be needed in Extras packages.

The various module files included in this package generate lots of rpmlint
errors about non-executable-scripts, which could be eliminated by removing the
shellbangs from the .pm files in %prep.
Comment 3 Bill Nottingham 2006-04-07 11:53:09 EDT
Heh. Wonder where I got my TableExtract from.

In the meantime, new stuff uploaded.
Comment 4 Bill Nottingham 2006-04-07 16:16:11 EDT
perl-HTML-TableExtract added for review in bug 188293.
Comment 5 Paul Howarth 2006-04-10 06:11:59 EDT
Please bump the release number for package revisions during the review phase -
it helps make clear which comments relate to which version of the package.

I'm unable to extract the current SRPM:
perl-Finance-Quote-1.11-1/cpio: premature end of file

The file I have is 89583 bytes, so I appear to have all of what's on the server.

Please ensure that perl(HTML::TableExtract) is a buildreq for this package if
you haven't already done so.
Comment 6 Bill Nottingham 2006-04-10 11:24:50 EDT
How is it a buildreq? I'll admit, I'm not 100% up on the perl build processes.

-2 uploaded.
Comment 7 Paul Howarth 2006-04-10 11:48:11 EDT
(In reply to comment #6)
> How is it a buildreq? I'll admit, I'm not 100% up on the perl build processes.

Strictly speaking it isn't - yet. The Makefile.PL has it as a prerequisite and
without it, you get this during the build:

Checking if your kit is complete...
Looks good
Warning: prerequisite HTML::TableExtract 0 not found.
Writing Makefile for Finance::Quote

Normally that would spell imminent doom when it came to running the test suite
(HTML::TableExtract would be needed in order to run any meaningful test), but in
this case you get away with it because there isn't actually any test suite.
Adding the buildreq now would make the package "ready" for when upstream
actually provides a test suite and is just good practice anyway.

The find/while/read/grep/mv loop in %prep could be simplified to:
find . -name *.pm | xargs %{__sed} -i -e '/^#!.*\/usr\/bin\/perl/d'

There is no changelog entry for the -2 revision.

Other than that, looks good.
Comment 8 Bill Nottingham 2006-04-10 16:45:07 EDT
-3 uploaded with buildreq, sed fix, and changelog.
Comment 9 Paul Howarth 2006-04-11 05:26:51 EDT
Review:

- rpmlint clean
- package and spec naming OK
- package meets guidelines
- license is GPL, matches spec, text included
- spec file written in English and is legible
- sources match upstream
- package builds ok in mock for rawhide (i386)
  (with perl-HTML-TableExtract in local repo for now)
- buildteqs OK
- no locales, libraries, pkgconfigs, or subpackages to worry about
- not relocatable
- no directory ownership or permissions issues
- no duplicate files
- %clean section present and correct
- macro usage is consistent
- code, not content
- no large docs
- docs don't affect runtime
- no .desktop file needed
- module appears to retrieve stock prices correctly
- no scriptlets

Approved.
Comment 10 Bill Nottingham 2006-04-19 12:18:01 EDT
Built. 
Comment 11 Bill Nottingham 2007-06-12 00:01:14 EDT
Package Change Request
----------------------
Package: perl-Finanace-Quote
New Branches: EL-4 EL-5
Comment 12 Jason Tibbitts 2007-06-12 16:12:13 EDT
CVS done.  BTW, there's no need to reopen old tickets to make CVS requests.

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