[Bug 619485] Review Request: python-kitchen - Small, useful pieces of code to make python coding easier

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=619485

Toshio Ernie Kuratomi <a.badger at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |a.badger at gmail.com
         AssignedTo|nobody at fedoraproject.org    |a.badger at gmail.com

--- Comment #2 from Toshio Ernie Kuratomi <a.badger at gmail.com> 2010-07-29 15:01:38 EDT ---
Good:
* Package name is fine (but see Needswork for version note)
* Spec file name matches package name
* License, LGPLv2+ is good and spec matches source
* Builds in mock.  See all the changes made below
* rpmlint has some false positives about spelling but otherwise is fine.
* spec file is readable, American English
* source matches upstream
* Builds in mock
* No locales at present.  Note that the package is setup for translations so in
  the future, you might need to use %find_lang
  https://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files
* No elf shared libraries
* Not relocatable
* All created files and directories are owned and nothing else
* No files listed multiple times
* Permissions set appropriately
* Macros used consistently
* Package contains code
* Nothing in %doc is needed at runtime
* Not a GUI app
* All filenames are valid UTF8

Needswork:
* Need to include COPYING.LESSER as well as COPYING -- the former is the
LGPLv2+ text.
  - Could also include NEWS

* Version must not include alphabetic characters.  Change to this:

Version: 0.2
Release: 0.3.a1%{?dist}

* Upstream includes unittests.  We should run them.  Note, some bugs were fixed
  in bzr for failing unittests on python-2.3 and python-2.4.  I can spin a new
  upstream release so we can build on RHEL-5 and RHEL-4::

BuildRequires: python-nose
[...]
%check
# In current mock, the PATH isn't being reset.  This causes failures in some
# subprocess tests as a check tests /root/bin/PROGRAM and fails with Permission
# Denied instead of File Not Found.  reseting the PATH works around this.
PATH=/bin:/usr/bin
nosetests

* There's an optional dependency on python-chardet.  I would require that as it
* adds good value to what the code can do::

BuildRequires: python-chardet
Requires: python-chardet

* There's a lot of documentation for kitchen available in the docs
  subdirectory.  We should include that.  (We can also build the documentation
  into html using python-sphinx but that's up to you.)

Minor:
* The description is more than 80 characters wide.  Reformatted::

kitchen includes functions to make gettext easier to use, handling unicode
text easier (conversion with bytes, outputting xml, and calculating how many
columns a string takes), and compatibility modules for writing code that uses
python-2.7 modules but needs to run on python-2.3.

* BuildRequires: python2-devel is better than python-devel in case we ever move
to python3 as the default python.

I'll attach a patch for these issues

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.