Bug#901548: diffoscope: Add lz4 comparator

[Xavier Briand]

Hi Mattia,

Comments bellow

On Thu, 14 Jun 2018 at 14:14 Mattia Rizzolo <mattia at debian.org> wrote:

> 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?
>

Totally, I'll fix it.


>
> +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).
>

I bluntly copy/pasted this from the gzip or xz comparator. I'll check 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?
>

Will do.


>
> --
> 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  `-
>

Thanks for the feedback.
-- 


Xavier Briand
Senior Web Application Developer| ExperiencePoint
<http://www.experiencepoint.com/>

20 Duncan Street, Suite 200, Toronto, ON, M5H 3G8
(416) 369-9888 x259

Connect with me: LinkedIn <http://www.linkedin.com/in/xavierbriand> |
Twitter <http://twitter.com/@experiencepoint> | Blog
<http://blog.experiencepoint.com/>

......................................................................


*ExperiencePoint is an Edison Award winning company.*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/reproducible-builds/attachments/20180614/0491bb51/attachment.html>