Bug 181979 - Review Request: python-geoip - Python bindings for the GeoIP geographical lookup libraries
Review Request: python-geoip - Python bindings for the GeoIP geographical loo...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-02-18 05:13 EST by Michael Fleming
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-02-21 17:38:09 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Michael Fleming 2006-02-18 05:13:47 EST
Spec Name or Url: http://www.enlartenment.com/extras/python-geoip.spec
SRPM Name or Url: http://www.enlartenment.com/extras/python-geoip-1.2.1-1.src.rpm
Description: 
This package contains the Python bindings for the GeoIP API, allowing IP to
location lookups to country, city and organization level within Python code
Comment 1 Jason Tibbitts 2006-02-19 11:06:34 EST
A clean package, builds fine in mock on i386 and x86_64.  However, I don't think
the name is proper.  By my understanding of the naming guidelines, your package
should be python-GeoIP.

rpmlint says:
W: python-geoip doc-file-dependency /usr/share/doc/python-geoip-1.2.1/test.py
/usr/bin/python
W: python-geoip doc-file-dependency
/usr/share/doc/python-geoip-1.2.1/test_city.py /usr/bin/python
W: python-geoip doc-file-dependency
/usr/share/doc/python-geoip-1.2.1/test_org.py /usr/bin/python
W: python-geoip doc-file-dependency
/usr/share/doc/python-geoip-1.2.1/test_region.py /usr/bin/python

This is the rpm dependency generator picking up a /usr/bin/python dependency
from the executable scripts marked %doc.  In this case I believe the added
dependency is just redundant since python is required anyway and there's no
prohibition against executable documentation that I can fine, but if you want to
quiet rpmlint, just chmod -x the files.

Note also that three of the example files will not work at all with the (free)
GeoIP package in Extras.  I don't think this is a blocker, but it may generate
bug reports.
Comment 2 Michael Fleming 2006-02-19 17:47:35 EST
Respin now available :-)

Spec Name or Url: http://www.enlartenment.com/extras/python-GeoIP.spec
SRPM Name or Url: http://www.enlartenment.com/extras/python-GeoIP-1.2.1-2.src.rpm

- Name changed per package naming conventions
- Execute bit removed from test*.py to keep RPM's dep generator from doing Bad
Things.
Comment 3 Jason Tibbitts 2006-02-20 22:01:18 EST
Cool.  rpmlint is now silent and the name neets the guidelines.
The specfile is taken from the Python template and is clean.
There are no installed (non-doc) .py or .pyc files, so no .pyo files to ghost.
The source file matches upstream.
The license is appropriate, matches the spec and is included as %doc.

Absolutely minor nitpick: your Description: needs a period.
Somewhat less minor nitpick: Requires: GeoIP is redundant.  RPM picks up the
libGeoIP.so.1 requirement automatically.

Sorry I didn't catch those at first glance.  Fix them up and it's good to go.
Comment 4 Michael Fleming 2006-02-21 03:52:30 EST
Spec Name or Url: http://www.enlartenment.com/extras/python-GeoIP.spec
SRPM Name or Url: http://www.enlartenment.com/extras/python-GeoIP-1.2.1-3.src.rpm

Recommendations noted and updates made.
Comment 5 Jason Tibbitts 2006-02-21 09:32:23 EST
Looks great.  Approved.
Comment 6 Michael Fleming 2006-02-21 17:38:09 EST
Imported and a devel branch build requested.

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