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@xxxxxxxxx> 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review