<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/121342/">https://git.reviewboard.kde.org/r/121342/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On dicembre 4th, 2014, 9:28 p.m. UTC, <b>Raphael Kubo da Costa</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This means Ark won't built with libarchive 2.8 anymore, right? If so, I'd rather do it for frameworks and leave the KDE4 version supporting a wider range of libarchive releases.</p></pre>
</blockquote>
<p>On dicembre 4th, 2014, 11:24 p.m. UTC, <b>Elvis Angelaccio</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Yes, that's right. Should I commit on <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">frameworks</code> then?</p></pre>
</blockquote>
<p>On dicembre 5th, 2014, 12:37 p.m. UTC, <b>Raphael Kubo da Costa</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think we can do a broader API update here. For example, that same wiki page says the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">ARCHIVE_COMPRESSION_*</code> macros are not part of libarchive3 (I still have them defined here for compatibility, though), so there's room for more improvements.</p></pre>
</blockquote>
<p>On dicembre 5th, 2014, 2:40 p.m. UTC, <b>Elvis Angelaccio</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That's fine for me. Feel free to discard this review, it makes sense to create a new one later.</p></pre>
</blockquote>
<p>On dicembre 5th, 2014, 3:07 p.m. UTC, <b>Raphael Kubo da Costa</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm fine with updating this review once you have the bandwidth to work on it instead of opening a new one, after all my suggestion is just an extension of the original idea behind this patch.</p></pre>
</blockquote>
<p>On dicembre 24th, 2014, 3:55 p.m. UTC, <b>Ivailo Monev</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You need to update the FindLibArchive cmake module to look for the new API too.</p></pre>
</blockquote>
<p>On dicembre 24th, 2014, 5:51 p.m. UTC, <b>Elvis Angelaccio</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks, updated now.</p></pre>
</blockquote>
<p>On gennaio 4th, 2015, 2:11 a.m. UTC, <b>Ivailo Monev</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There are additional checks in ark/plugins/libarchive/CMakeLists.txt like HAVE_LIBARCHIVE_CAB_SUPPORT and HAVE_LIBARCHIVE_RPM_SUPPORT. Also, you can drop the CLI plugins for Zip and 7z formats and update the MIME types that are supported by the libarchive plugin as these formats are supported since 3.0.0. It may be too much for a single review request but I just want to let you know.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks, indeed there are some checks still to be removed. I'm not sure if I am supposed to reopen this review or create a new one (let's wait what Raphael says).
Regarding the 7z and zip formats in libarchive, yes it's something that needs a proper discussion. For example, I have another review request for the creation of password protected archives. AFAIK, that feature would not be possible by using libarchive.</p></pre>
<br />
<p>- Elvis</p>
<br />
<p>On gennaio 2nd, 2015, 4:39 p.m. UTC, Elvis Angelaccio wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Utils and Raphael Kubo da Costa.</div>
<div>By Elvis Angelaccio.</div>
<p style="color: grey;"><i>Updated Gen. 2, 2015, 4:39 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ark
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch replaces the deprecated <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">libarchive</code> functions used in <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">libarchivehandler.cpp</code> with their replacements.
Quick reference: https://github.com/libarchive/libarchive/wiki/ManPageLibarchiveChanges3#Deprecated_Symbols</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">After compiling all the warnings about deprecated symbols are gone.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(b17c30b4f0f57db6e6cc1e8ee77c5fe5e8b32e77)</span></li>
<li>cmake/modules/COPYING-CMAKE-SCRIPTS <span style="color: grey">(4b417765f3a834ce6b0a216f6b6c0fe2d3f0bed5)</span></li>
<li>cmake/modules/FindLibArchive.cmake <span style="color: grey">(db14d7b279e67039c454fcaa82a21885feec26e0)</span></li>
<li>plugins/libarchive/libarchivehandler.cpp <span style="color: grey">(0fba52864523bf07ba8c898aa6b15cd867001de6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121342/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>