Bug 565502 - Review Request: python-recaptcha-client - A plugin for reCAPTCHA and reCAPTCHA Mailhide
Summary: Review Request: python-recaptcha-client - A plugin for reCAPTCHA and reCAPTCH...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dave Malcolm
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-02-15 14:12 UTC by Stephen Gallagher
Modified: 2010-04-02 02:41 UTC (History)
4 users (show)

Fixed In Version: python-recaptcha-client-1.0.5-3.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-03-16 15:48:53 UTC
Type: ---
dmalcolm: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Stephen Gallagher 2010-02-15 14:12:10 UTC
Spec URL: http://fedorapeople.org/~sgallagh/packagereview/python-recaptcha-client.spec
SRPM URL: http://fedorapeople.org/~sgallagh/packagereview/python-recaptcha-client-1.0.5-0.fc12.src.rpm
Description:
Provides a CAPTCHA for Python using the reCAPTCHA service. Does not require any imaging libraries because the CAPTCHA is served directly from reCAPTCHA. Also allows you to securely obfuscate emails with Mailhide. This functionality requires pycrypto. This library requires two types of API keys. If you'd like to use the CAPTCHA, you'll need a key from http://recaptcha.net/api/getkey. For Mailhide, you'll need a key from http://mailhide.recaptcha.net/apikey.

Comment 1 Stephen Gallagher 2010-02-15 14:14:38 UTC
Additional information: this feature is being packaged because it is a dependency for the next major release of ReviewBoard (version 1.5), which is already included in Fedora.

Comment 2 Dave Malcolm 2010-02-15 22:13:05 UTC
(Not sure I'll have time for a full review, am getting ready for PyCon; but here are some things I spotted)

The URL seems to be for recaptcha as a whole, and I found it hard to find info on the client; I suggest changing the URL field to this:
  http://pypi.python.org/pypi/recaptcha-client

Initial review of specfile seems sane, with some issues noted below:

Please do a scratch build in Koji.

Please run rpmlint on the packages; looks like the %description needs to be line-wrapped, at least:

python-recaptcha-client.noarch: E: description-line-too-long Provides a CAPTCHA for Python using the reCAPTCHA service. Does not require any imaging libraries because the CAPTCHA is served directly from reCAPTCHA. Also allows you to securely obfuscate emails with Mailhide. This functionality requires pycrypto. This library requires two types of API keys. If you'd like to use the CAPTCHA, you'll need a key from http://recaptcha.net/api/getkey. For Mailhide, you'll need a key from http://mailhide.recaptcha.net/apikey.
python-recaptcha-client.noarch: W: invalid-license X11
python-recaptcha-client.noarch: W: no-documentation
1 packages and 0 specfiles checked; 1 errors, 2 warnings.

description-line-too-long: please fix this
python-recaptcha-client.noarch: W: invalid-license X11: is there any license information on this code other that the reference in the setup.py and on the pypi web site?
python-recaptcha-client.noarch: W: no-documentation:  I don't see any documentation in the tarball, so I think this can be waived


BTW, which releases are you targetting?  Rawhide and EPEL5?

Comment 3 Thomas Spura 2010-02-15 22:45:51 UTC
There are also some other issues:

- use %global and not %define
https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

- Buildrequires/requires: python is added automatic

- Could you please be a bit more specific in %files?
  When you use %{python_sitelib}/recaptcha_client-*.egg-info and %{python_sitelib}/recaptcha you are notified if the egg can't be build.

- Where is this spec file based on?
  When you run "rpmdev-newspec -t python python-recaptcha-client" you could use that spec file as a starting point.

e.g. from %install
%{__python} setup.py install -O1 --skip-build --root %{buildroot}

You don't have this --skip-build in your spec. Please add this, because this is part of convention in all python packages.
https://fedoraproject.org/wiki/Packaging:Python

Comment 5 Dave Malcolm 2010-03-11 16:30:24 UTC
A few minor nits:
- remove the "# sitelib for noarch packages, sitearch for others (remove the unneeded one)" comment, and just use sitelib.  FWIW, these aren't actually needed in Fedora 13 onwards; see:
http://fedoraproject.org/wiki/Packaging/Python#Macros
on how to conditionalize them
- in the %build stanza, remove the comment about removing CFLAGS, and actually remove the CFLAGS :)
- the comment in the %files stanza is probably redundant
- in future %changelog entries, it can be better to briefly say what the changes were, rather than "specfile changes"
- the Summary says "A plugin for reCAPTCHA and reCAPTCHA Mailhide", and I find this wording rather awkward.  If I'm reading it right "reCAPTCHA and reCAPTCHA Mailhide" is the functionality being added, but what is the program to which the functionality is being added?  If it's "python", then the normal wording would be "Python module"

Comment 7 Dave Malcolm 2010-03-11 17:12:49 UTC
Looking good, but you didn't rerun rpmlint; this shows a problem:
python-recaptcha-client.src:60: W: macro-in-%changelog %files
2 packages and 0 specfiles checked; 0 errors, 1 warnings.

APPROVED, but please fix the above issue by escaping that reference to "%files" to "%%files" when you import the package, and ensure that rpmlint is thus clean.

Comment 8 Stephen Gallagher 2010-03-11 17:22:43 UTC
New Package CVS Request
=======================
Package Name: python-recaptcha-client
Short Description: Python module for reCAPTCHA and reCAPTCHA Mailhide
Owners: sgallagh
Branches: F-12 F-13 EL-5
InitialCC:

Comment 9 Jason Tibbitts 2010-03-11 19:28:01 UTC
Somehow this review was never assigned to anyone.  Please take review tickets when you intend to review them.

Comment 10 Jason Tibbitts 2010-03-11 19:28:47 UTC
CVS done (by process-cvs-requests.py).

Comment 11 Dave Malcolm 2010-03-11 19:38:49 UTC
(In reply to comment #9)
> Somehow this review was never assigned to anyone.  Please take review tickets
> when you intend to review them.    
My bad; sorry about that

Comment 12 Fedora Update System 2010-03-16 15:48:20 UTC
python-recaptcha-client-1.0.5-3.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/python-recaptcha-client-1.0.5-3.el5

Comment 13 Fedora Update System 2010-04-02 02:41:20 UTC
python-recaptcha-client-1.0.5-3.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.


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