Bug#901548: diffoscope: Add lz4 comparator

Mattia Rizzolo mattia at debian.org
Thu Jun 14 19:14:37 BST 2018


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 <lunar at 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  `-
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/reproducible-builds/attachments/20180614/1f895e25/attachment.sig>


More information about the Reproducible-builds mailing list