<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/124639/">https://git.reviewboard.kde.org/r/124639/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 6th, 2015, 9:59 a.m. UTC, <b>Konrad Zemek</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;">Actually, there is a bug here. We require at least TagLib 1.7, but after your commit that will change to 1.7.1 . <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">taglib-config --version</code> returns <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">1.7.0</code> for TagLib 1.7, and while <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">"1.7" STRLESS "1.7.0"</code> is true, <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">"1.7" VERSION_LESS "1.7.0"</code> is false.</p></pre>
</blockquote>
<p>On August 6th, 2015, 8:14 p.m. UTC, <b>Omar Plummer</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 not sure I follow, so please bear with me if I'm missing something simple.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Isn't "1.7" technically equivalent to "1.7.0"? In which case "1.7" < "1.7.0" should in fact be false?</p></pre>
</blockquote>
<p>On August 6th, 2015, 8:47 p.m. UTC, <b>Konrad Zemek</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 correct, and it is in fact false. :) That's why after your change, when compiled on a system with taglib 1.7, the build will fail with <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">-- TagLib version too old: version searched :1.7, found 1.7.0</code></p></pre>
</blockquote>
<p>On August 6th, 2015, 9:16 p.m. UTC, <b>Omar Plummer</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;">Ahh.... I see your point.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Would the appropriate approach for this be to change 'set(TAGLIB_MIN_VERSION "1.7")' to 'set(TAGLIB_MIN_VERSION "1.7.0") in addition then?</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;">Actually, never mind :) ... I just realised that I didn't quite in fact get the point.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The thing to do here would be to change 'TAGLIB_VERSION VERSION_LESS "${TAGLIB_MIN_VERSION}"' to '"${TAGLIB_MIN_VERSION}" VERSION_GREATER TAGLIB_VERSION'</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Should I do another review request, or is it possible/acceptable to attach the new patch to this one?</p></pre>
<br />
<p>- Omar</p>
<br />
<p>On August 6th, 2015, 12:57 a.m. UTC, Omar Plummer 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 Amarok.</div>
<div>By Omar Plummer.</div>
<p style="color: grey;"><i>Updated Aug. 6, 2015, 12:57 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=351013">https://bugs.kde.org/show_bug.cgi?id=351013</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
amarok
</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;">Build fails with the following error:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">-- TagLib version too old: version searched :1.7, found 1.10.0</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>cmake/modules/FindTaglib.cmake <span style="color: grey">(76b83ac)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124639/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>