Hi Mattia, Comments bellow
On Thu, 14 Jun 2018 at 14:14 Mattia Rizzolo <mat...@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 <lu...@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.*