Code review comment for lp:~ev/apt-clone/python3

Revision history for this message
Colin Watson (cjwatson) wrote :

> === modified file 'apt_clone.py'
> --- apt_clone.py 2012-01-27 17:14:05 +0000
> +++ apt_clone.py 2012-06-11 15:50:47 +0000
> @@ -182,7 +184,8 @@
> tarinfo = tarfile.TarInfo("./var/lib/apt-clone/installed.pkgs")
> tarinfo.size = len(s)
> tarinfo.mtime = time.time()
> - tar.addfile(tarinfo, StringIO(s))
> + s = s.encode('utf-8')
> + tar.addfile(tarinfo, BytesIO(s))

tarinfo.size will be wrong if s isn't entirely ASCII. I think you need
to encode earlier.

> - #print tar.getnames()
> + print(tar.getnames())

Did you mean to uncomment this?

> === modified file 'debian/changelog'
> --- debian/changelog 2012-01-27 16:54:20 +0000
> +++ debian/changelog 2012-06-11 15:50:47 +0000
> @@ -8,6 +8,16 @@
>
> -- Michael Vogt <email address hidden> Fri, 27 Jan 2012 16:29:17 +0100
>
> +apt-clone (0.2.2ubuntu1) UNRELEASED; urgency=low
> +
> + * Port to Python 3:
> + - Use Python 3-style print functions.
> + - Use "raise Exception(value)" syntax rather than the old-style "raise
> + Exception, value".
> + - Use dict.items() rather than dict.iteritems().
> +
> + -- Colin Watson <email address hidden> Mon, 11 Jun 2012 09:12:14 +0100
> +
> apt-clone (0.2.2) unstable; urgency=low
>
> * fix extraction of no-longer downloadable debs, thanks
>

This might need updating :-)

The rest looks good to me.

 review needs-fixing

review: Needs Fixing

« Back to merge proposal