Bug#901548: diffoscope: Add lz4 comparator
I opened a merge request on Gitlab ( https://salsa.debian.org/reproducible-builds/diffoscope/merge_requests/4) forwarded 901548 https://salsa.debian.org/reproducible-builds/diffoscope/merge_requests/4 On Thu, 14 Jun 2018 at 14:30 Mattia Rizzolo wrote: > On Thu, Jun 14, 2018 at 02:25:09PM -0400, Xavier Briand wrote: > > I followed the instructions provided in the CONTRIBUTING.rst > > < > https://salsa.debian.org/reproducible-builds/diffoscope/blob/master/CONTRIBUTING.rst > > > > file. > > That being said, I'd feel more comfortable using gitlab's merge request. > > > > How would I go about it? Close this one and submit an other one? > > No, please keep this open and cross reference them all (here we'll mark > this bug as forwarded to the MR, MR will just link this, and the commit > message should have a 'Closes: #901548' line). > > -- > 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 `- > -- 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.*
Bug#901548: diffoscope: Add lz4 comparator
Hi Mattia, Comments bellow On Thu, 14 Jun 2018 at 14:14 Mattia Rizzolo 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 > > 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.*
Bug#901548: diffoscope: Add lz4 comparator
Hi Chris, I followed the instructions provided in the CONTRIBUTING.rst <https://salsa.debian.org/reproducible-builds/diffoscope/blob/master/CONTRIBUTING.rst> file. That being said, I'd feel more comfortable using gitlab's merge request. How would I go about it? Close this one and submit an other one? Cheers On Thu, 14 Jun 2018 at 14:05 Chris Lamb wrote: > tags 901548 + patch > thanks > > Dear Xavier, > > Thanks! I was just wondering if you considered submitting this as a > "merge request" on our Gitlab instance? > > https://salsa.debian.org/reproducible-builds/diffoscope > > :) > > > Best wishes, > > -- > ,''`. > : :' : Chris Lamb > `. `'` la...@debian.org / chris-lamb.co.uk >`- > -- 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.*
Bug#901548: diffoscope: Add lz4 comparator
Source: diffoscope Version: 95 Severity: wishlist Add lz4 comparator. -- 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.* 0001-Add-lz4-comparator.patch Description: Binary data
