Control: tag -1 moreinfo Hi Xavier,
On Thu, Jun 14, 2018 at 12:27:47PM -0400, Xavier Briand wrote: > Add lz4 comparator. Thanks for your patch! However, I'd like to ask for some changes and improvements, and I also have some questions about it. See the comments inlined in the diff: --- /dev/null +++ b/diffoscope/comparators/lz4.py @@ -0,0 +1,61 @@ +# -*- coding: utf-8 -*- +# +# diffoscope: in-depth comparison of files, archives, and directories +# +# Copyright © 2014-2015 Jérémy Bobbio <lu...@debian.org> Surely this is a copy-paste error of some sort? +class Lz4File(File): + DESCRIPTION = "LZ4 compressed files" + CONTAINER_CLASS = Lz4Container + FILE_TYPE_RE = re.compile(r'^LZ4 compressed data$') Is this regex actually correct? I don't have any .lz4 file at hand, but I doubt `file` returns that string and nothing else (i.e., I'm concerned about the anchoring). Then, we really like tests. They are as easy as the comparators to write, and there are plenty of examples, do you think you could provide some? -- regards, Mattia Rizzolo GPG Key: 66AE 2B4A FCCF 3F52 DA18 4D18 4B04 3FCD B944 4540 .''`. more about me: https://mapreri.org : :' : Launchpad user: https://launchpad.net/~mapreri `. `'` Debian QA page: https://qa.debian.org/developer.php?login=mattia `-
signature.asc
Description: PGP signature