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.
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.
(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?
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
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 Koji scratch builds completed successfully: http://koji.fedoraproject.org/koji/taskinfo?taskID=2041826 (Fedora 13) http://koji.fedoraproject.org/koji/taskinfo?taskID=2041845 (EPEL 5)
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"
Thanks for the review. Updated spec and srpm: Spec URL: http://fedorapeople.org/~sgallagh/packagereview/python-recaptcha-client.spec SRPM URL: http://fedorapeople.org/~sgallagh/packagereview/python-recaptcha-client-1.0.5-2.fc12.src.rpm Built successfully in Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2046956
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.
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:
Somehow this review was never assigned to anyone. Please take review tickets when you intend to review them.
CVS done (by process-cvs-requests.py).
(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
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
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.