<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/103423/">http://git.reviewboard.kde.org/r/103423/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 22nd, 2011, 6:39 p.m., <b>Rex Dieter</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;">Looking good, a few comments:
I'd recommend using
if("${_isSystem}" LESS 0)
instead of a strict -1 match. Matches other cmake code that does the same thing.
Speaking of other cmake code doing similar things... I've never seen them check against anything other than CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES, what's the purpose of adding CMAKE_CXX_IMPLICIT_LINK_DIRECTORIES here too?</pre>
</blockquote>
<p>On December 22nd, 2011, 7:15 p.m., <b>Harald Sitter</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;">it be unacceptable anyway as this part is stolen from the empire of kdelibs, so the fix needs to be done there and pulled in</pre>
</blockquote>
<p>On December 22nd, 2011, 7:20 p.m., <b>Rex Dieter</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;">I take my first comment back, kdelibs' does indeed use STREQUAL "-1" , ok then.
Given that, I'd support simply stealing the kdelibs' code in FindKDE4Internal.cmake , (that matches CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES only).</pre>
</blockquote>
<p>On December 22nd, 2011, 8:37 p.m., <b>Felix Geyer</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;">The review request for kdelibs is at https://git.reviewboard.kde.org/r/103422/</pre>
</blockquote>
<p>On February 8th, 2012, 4:27 p.m., <b>Rex Dieter</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;">Imo, even without the CMAKE_CXX_IMPLICIT_LINK_DIRECTORIES bits, that's better than the status quo, which essentially forces everyone (who care about superfluous rpath'ing) to patch this or use some other hammer to strip out the rpath'ing going on.</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;">in short, don't let 'perfect' be the enemy of the good.</pre>
<br />
<p>- Rex</p>
<br />
<p>On December 15th, 2011, 10:17 p.m., Felix Geyer wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Phonon.</div>
<div>By Felix Geyer.</div>
<p style="color: grey;"><i>Updated Dec. 15, 2011, 10:17 p.m.</i></p>
<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;">Set CMAKE_INSTALL_RPATH only when phonon isn't installed into a system lib dir.</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/FindPhononInternal.cmake <span style="color: grey">(2bad949)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103423/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>