<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/119339/">https://git.reviewboard.kde.org/r/119339/</a>
</td>
</tr>
</table>
<br />
<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 don't have any objections with forward-porting the kdelibs commit to e-c-m, but thought it wasn't worth the effort since FindPyQt4.cmake is not part of any e-c-m release and it would make more sense to write a new version from scratch were we to move it out of <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Attic/</code>. The forward-port doesn't hurt, though, so go ahead if you will.</p></pre>
<br />
<p>- Raphael Kubo da Costa</p>
<br />
<p>On July 17th, 2014, 5:54 p.m. EEST, Scott Kitterman 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 Extra Cmake Modules, Alex Merry, Luca Beltrame, Raphael Kubo da Costa, and Simon Edwards.</div>
<div>By Scott Kitterman.</div>
<p style="color: grey;"><i>Updated July 17, 2014, 5:54 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
extra-cmake-modules
</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;">Since PyQt 4.10, PyQt.pyqtconfig is deprecated and not available unless PyQt is built using the old configure script.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There is no direct replacement for it, as PyQt's new build system does not provide as much information as before by design. Luckily, most of the variables we are interested in can be obtained from PyQt's QtCore module itself even if its old build system is used.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The only exception is pyqt_sip_dir, which cannot be determined at all if pyqtconfig is not available. In this case, the most we can do is guess the default path like QScintilla2 does, and fail if it does not exist.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
The user then needs to specify it manually via CMake with something like -DPYQT4_SIP_DIR=/usr/share/sip/PyQt4. To this effect, all variables set by FindPyQt4.cmake have been made cache variables, which means their values can be overriden by the user, thus ignoring the contents read via FindPyQt.py.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is identical to http://commits.kde.org/kdelibs/a7e47438d4e3469dc9df70d613826cb360fc8d19 which was review https://git.reviewboard.kde.org/r/119302/ . We should make the same change in e-c-m prior to work to make the module work for either PyQt4 or PyQt5.</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;">No additional testing done for this change, but it's identical to the kdelibs change.</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;">
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119339/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>