[Bug 790154] Review Request: python-mwlib - MediaWiki parser and utility library

[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=790154

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |a.badger at gmail.com
         AssignedTo|nobody at fedoraproject.org    |a.badger at gmail.com
               Flag|                            |fedora-review+

--- Comment #1 from Toshio Ernie Kuratomi <a.badger at gmail.com> 2012-02-13 15:28:02 EST ---
There's a few needswork items but I'll APPROVE this if you fix those when you
import the package.

Good:
* Matches naming guidelines
* License is BSD in source and spec
* License is spelled out in full in the README.rst file which is included in
  the package
* Spec file is legible
* Source matches upstream
* No locale files that need to be handled
* No bundled libraries found
* Package not relocatable
* Package owns all directories it creates and no more.
* Proper permissions for files
* Macros used consistently
* Code, not content
* No large documentation
* Nothing in %doc affects application at runtime
* Not an elf library
* Not a GUI application
* All filenames are valid utf-8

Needswork:

* package doesn't build in koji -- needs:
    BuildRequires: python-setuptools
  - Tested that the package builds with that change made
* Package needs Requires: python-setuptools since it uses import pkg_resources
  in the code


Cosmetic:

* rpmlint just has warnings for no man pages:
  python-mwlib.x86_64: W: no-manual-page-for-binary mw-post (and all the other
  %{_bindir} scripts).  These are warnings and if no man pages are easily
  available, ignorable.

Potential future issues:

* The package installs a toplevel argv.py file/module.  This seems to just be
  used for the command line scripts for parsing the command line.  Since this
  has the potential to pollute the python package namespace with a somewhat
  common name, suggest to upstream that they may want to restructure like
  this::
    mv argv.py mwlib/._argv.py
    sed s/import argv/from mwlib import _argv as argv/

-- 
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.