<div dir="ltr">Hi Mattia,<div><br></div><div>Comments bellow<br><br><div class="gmail_quote"><div dir="ltr">On Thu, 14 Jun 2018 at 14:14 Mattia Rizzolo <<a href="mailto:mattia@debian.org">mattia@debian.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Control: tag -1 moreinfo<br>
<br>
Hi Xavier,<br>
<br>
On Thu, Jun 14, 2018 at 12:27:47PM -0400, Xavier Briand wrote:<br>
> Add lz4 comparator.<br>
<br>
Thanks for your patch!<br>
<br>
However, I'd like to ask for some changes and improvements, and I also<br>
have some questions about it.<br>
<br>
See the comments inlined in the diff:<br>
<br>
--- /dev/null<br>
+++ b/diffoscope/comparators/lz4.py<br>
@@ -0,0 +1,61 @@<br>
+# -*- coding: utf-8 -*-<br>
+#<br>
+# diffoscope: in-depth comparison of files, archives, and directories<br>
+#<br>
+# Copyright © 2014-2015 Jérémy Bobbio <<a href="mailto:lunar@debian.org" target="_blank">lunar@debian.org</a>><br>
<br>
Surely this is a copy-paste error of some sort?<br></blockquote><div><br></div><div>Totally, I'll fix it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
+class Lz4File(File):<br>
+ DESCRIPTION = "LZ4 compressed files"<br>
+ CONTAINER_CLASS = Lz4Container<br>
+ FILE_TYPE_RE = re.compile(r'^LZ4 compressed data$')<br>
<br>
Is this regex actually correct? I don't have any .lz4 file at hand, but<br>
I doubt `file` returns that string and nothing else (i.e., I'm concerned<br>
about the anchoring).<br></blockquote><div><br></div><div>I bluntly copy/pasted this from the gzip or xz comparator. I'll check the anchoring.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Then, we really like tests. They are as easy as the comparators to<br>
write, and there are plenty of examples, do you think you could provide<br>
some?<br></blockquote><div><br></div><div>Will do.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
-- <br>
regards,<br>
Mattia Rizzolo<br>
<br>
GPG Key: 66AE 2B4A FCCF 3F52 DA18 4D18 4B04 3FCD B944 4540 .''`.<br>
more about me: <a href="https://mapreri.org" rel="noreferrer" target="_blank">https://mapreri.org</a> : :' :<br>
Launchpad user: <a href="https://launchpad.net/~mapreri" rel="noreferrer" target="_blank">https://launchpad.net/~mapreri</a> `. `'`<br>
Debian QA page: <a href="https://qa.debian.org/developer.php?login=mattia" rel="noreferrer" target="_blank">https://qa.debian.org/developer.php?login=mattia</a> `-<br></blockquote><div><br></div><div>Thanks for the feedback. </div></div></div></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><table style="font-family:tahoma,verdana,arial,helvetica,sans-serif;font-size:11px;line-height:1.5em;color:rgb(88,89,91);padding:0pt 10px;border-left:5px solid rgb(209,211,212)" class="inbox-inbox-inbox-mce-item-table" border="0" cellpadding="0" cellspacing="0"><tbody><tr><td style="vertical-align:top"> </td><td><p style="margin:6px 0pt"><span style="font-size:16px;color:rgb(0,0,0)">Xavier Briand</span><br>Senior Web Application Developer| <a href="http://www.experiencepoint.com/" style="color:rgb(79,121,169)">ExperiencePoint</a><br></p><p style="margin:6px 0pt">20 Duncan Street, Suite 200, Toronto, ON, M5H 3G8<br> (416) 369-9888 x259</p><p>Connect with me: <a href="http://www.linkedin.com/in/xavierbriand"><span style="color:rgb(61,133,198)">LinkedIn</span></a> | <a href="http://twitter.com/@experiencepoint"><span style="color:rgb(61,133,198)">Twitter</span></a> | <a href="http://blog.experiencepoint.com/"><span style="color:rgb(61,133,198)">Blog</span></a><br></p><p style="font-size:8px;letter-spacing:0.5ex;line-height:1.2em">......................................................................</p></td></tr><tr><td> </td><td><p style="padding-top:5px"><i>ExperiencePoint is an Edison Award winning company.</i></p></td></tr></tbody></table></div></div>