<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/101568/">http://git.reviewboard.kde.org/r/101568/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 13th, 2011, 8:05 a.m., <b>Alexander Neundorf</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;">Looks good in general.
The foreach() loop uses new features from cmake 2.8.0, and KDE depends on cmake 2.6.4 (later on this year we will require probably cmake 2.8.6, then it will be fine), so this must not be used.
Can you change the foreach()-loop to the basic syntax ?
Also, the message() which is printed in the body of the loop shouldn't be there, a module should only print its results, all the rest should be for debugging only.
Alex</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;">Could you elaborate a bit on what is wrong with the foreach loop? According to the documentation for CMake 2.6 it also supports range-based for loops, at least that is what http://www.cmake.org/cmake/help/cmake2.6docs.html#command:foreach tells me.</pre>
<br />
<p>- Arjen</p>
<br />
<p>On June 10th, 2011, 7:18 p.m., Arjen Hiemstra 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 kdelibs and Sebastian Kügler.</div>
<div>By Arjen Hiemstra.</div>
<p style="color: grey;"><i>Updated June 10, 2011, 7:18 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;">As requested by Sebas, this patch adds a FindQtMobililty.cmake that can be used to find QtMobility related files. It has support for minimum versions and searching for individual components.</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;">There is a simple testcase included. Furthermore I have tested the minimum version and component related options with a local test file.</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-tests/QtMobility/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>cmake/modules/FindQtMobility.cmake <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101568/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>