[Bug 818264] Review Request: xlwt - Spreadsheet python 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=818264

--- Comment #6 from Alec Leamas <leamas.alec at gmail.com> 2012-05-03 09:50:32 EDT ---
(In reply to comment #4)
> I was already halfway through the review so I'll post it as it is and then just
> list still applicable stuff in another comment
OK 
[cut]

> [!]: MUST Package requires other packages for directories it uses.
> Package should probably require python (even though it is even in
> minimal install)
Done


> [!]: MUST Rpmlint output is silent.
It is, now.


> [!]: SHOULD If the source package does not include license text(s) as a
>      separate file from upstream, the packager SHOULD query upstream to
>      include it.
Done. https://github.com/python-excel/xlwt/issues/4


> [!]: SHOULD %check is present and all tests pass.
> 
> If possible checks available in tests/ directory should be run during %check
It's not, I have looked into that. It's just not what it seems to be ;)


> Issues:
> [!]: MUST Package contains no bundled libraries.
> 
> There is antlr-python bundled: xlwt/antlr.py
> this needs to be unbundled, antlr-python should be put into
> Requires. Shouldn't be hard and it should keep working with few/no
> modifications of source code. Bring it up with upstream as
> well. Bundling is ugly practice
Done. Link in spec, along with the patch. Good catch!


> [!]: SHOULD If the source package does not include license text(s) as a
>      separate file from upstream, the packager SHOULD query upstream to
>      include it.
> As stated above a full license text of LGPLv2.1 should probably be
> included (if that is indeed the intention of upstream with
> utils.py). Get in touch with upstream about this.
Done: https://github.com/python-excel/xlwt/issues/4


> [!]: MUST If (and only if) the source package includes the text of the
>      license(s) in its own file, then that file, containing the text of the
>      license(s) for the package is included in %doc.
> It would be good to contact upstream and get them to include full text
> of LGPL in the tarballs. I also see no reason to have licenses in a
> Python file, but I don't particularly care about that :-)
> 
> [!]: MUST License field in the package spec file matches the actual license.
[cut]
> Final License tag will most probably be something like:
> LGPLv2+ and BSD and BSD with attribution
> 
> But we should wait on legal with this.
OK, lets wait. I presume you handle the contacts with fedora-legal?!


> [!]: MUST Rpmlint output is silent.
Fixed, now silent.


> licenses.py should be converted to UTF-8 prior to installing
Fixed...


> I just noticed it would be nice to also change:
> %{python_sitelib}/*
> to:
> %{python_sitelib}/%{name}
> 
> So future updates don't include something accidentaly.
Not exactly that way, but fixed.

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