<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/122211/">https://git.reviewboard.kde.org/r/122211/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 22nd, 2015, 9:10 p.m. UTC, <b>Albert Astals Cid</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 disagree with this. What's your rationale for making the cmake code more complex?</p></pre>
</blockquote>
<p>On January 22nd, 2015, 9:30 p.m. UTC, <b>Andreas Sturmlechner</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;">Well, the same could be achieved with fewer lines:
Put find_package(Qt5Test...) behind if(BUILD_TESTING) in the root CMakeLists.txt, that would remove the need to patch the tests subdirs.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Do you disagree with making Qt5Test optional in principle or shall I go the simpler route?</p></pre>
</blockquote>
<p>On January 22nd, 2015, 9:33 p.m. UTC, <b>Albert Astals Cid</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;">Note i'm not the maintainer so me disagreeing doesn't mean someone will pick it up and commit it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I disagree since i don't see the benefit and to me tests are an integral part of the code.</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;">As for my rationale, I'd like to refer to previous discussions that resulted in favor of making tests optional: https://git.reviewboard.kde.org/r/117393/</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tests are important to developers and packagers, but users of source packages should not be forced to build the tests all the time. This is especially the case when pulling from git, where tests might hang or there is no (often required) X available at build time.</p></pre>
<br />
<p>- Andreas</p>
<br />
<p>On January 22nd, 2015, 9:06 p.m. UTC, Andreas Sturmlechner 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 Games.</div>
<div>By Andreas Sturmlechner.</div>
<p style="color: grey;"><i>Updated Jan. 22, 2015, 9:06 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
libkdegames
</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;">Qt5Test should not be mandatory, patch to add tests subdirs only if BUILD_TESTING and moved find_package(Qt5Test...) into the respective tests CMakeLists.txt</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I should add that I don't have write access, so I ask for someone else to merge it.</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;">Built with and without tests.</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">(2664fd4ea3234bfe6f9c7d4062579e91adc163fe)</span></li>
<li>libkdegamesprivate/CMakeLists.txt <span style="color: grey">(b122f37abe1b403042c347c5ad4ff5d37e3dffba)</span></li>
<li>libkdegamesprivate/tests/CMakeLists.txt <span style="color: grey">(59ac2b4be7d35eb9518d277379ad53a7f00e5171)</span></li>
<li>tests/CMakeLists.txt <span style="color: grey">(7b95e60cd11119b745e8c6830f1514a61fb0c6e1)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122211/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>