Code review comment for lp:~takluyver/ubuntu/quantal/python-tz/merge-py3

Revision history for this message
Barry Warsaw (barry) wrote :

Hi Thomas,

This is really great, and I appreciate your contribution to Ubuntu, especially because this adds Python 3 support to a very useful package. I was able to build it locally and try it out on both Python 2 and Python 3. Everything works great.

I get one warning when building the source package:

W: python-tz source: missing-license-paragraph-in-dep5-copyright public-domain (paragraph at line 27)

and I get one minor warning on the resulting .debs:

% lintian *.deb
W: python-tz: wrong-name-for-upstream-changelog usr/share/doc/python-tz/CHANGES.txt

The other thing I suggest is to structure your debian/rules file to be more in line with these recommendations:

http://wiki.debian.org/Python/LibraryStyleGuide

Pay attention specifically to the section on enabling the tests in all versions (which I see is currently disabled -- why?) and in avoiding the use of for-loops to build the Python 3 packages. If you fix these, I would happily sponsor your upload, but for Quantal only, as this is a new feature that isn't appropriate for SRU'ing into Precise.

review: Needs Fixing

« Back to merge proposal