<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/107489/">http://git.reviewboard.kde.org/r/107489/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Small CMake change is required, then this is OK to be shipped.
Though if possible, please also change the include guards of the header files, especially the exported ones. Imo TESTSUITE_H is too generic for a library header. Rather, I suggest prefixing them with e.g. KDEV_JSONTEST_ or similar - could you please do that as well? Then just commit this stuff.
Great work, thanks Olivier!</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/107489/diff/2/?file=96680#file96680line50" style="color: black; font-weight: bold; text-decoration: underline;">tests/CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">50</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">find_package(QJSON)</pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'd now move this check into the central CMakeLists.txt along with the macro_log_feature line from plugins/CMakeLists.txt, then only keep the if(QJSON_FOUND) conditionals in the sub-CMakeLists.txt</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/107489/diff/2/?file=96681#file96681line1" style="color: black; font-weight: bold; text-decoration: underline;">tests/json/CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">find_library(QJSON REQUIRED)</pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this is not required here (it was already required farther up in the cmake hierarchy)</pre>
</div>
<br />
<p>- Milian</p>
<br />
<p>On November 28th, 2012, 4:02 p.m., Olivier Jean de Gaalon 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 KDevelop.</div>
<div>By Olivier Jean de Gaalon.</div>
<p style="color: grey;"><i>Updated Nov. 28, 2012, 4:02 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;">This patch provides a framework to allow creating json tests for arbitrary objects.
In particular, this allows declarations to be tested using a simple DUChainVisitor (also included) which extracts json data from the declaration's comments and runs the tests specified therein.
This can be used for any language (see kdevelop patch for cpp's implementation).
Please pay special attention to cmake stuff when reviewing because I have no idea what I'm doing.
Why not C++11?
> We don't support it in KDevelop yet.
Why not Boost?
> NIH
Why do the json test functions need to be included instead of linked in?
> Because you'll add your own tests, which will be added to a different instantiation of TestSuite than the provided ones because they come from another library. C++11 might help here with the export keyword, but it's not really important.</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;">Who tests the testers?</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>tests/CMakeLists.txt <span style="color: grey">(4a306a4)</span></li>
<li>tests/json/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/json/declarationvalidator.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/json/declarationvalidator.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/json/delayedoutput.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/json/delayedoutput.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/json/jsondeclarationtests.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/json/jsonducontexttests.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/json/jsontesthelpers.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/json/jsontypetests.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/json/testsuite.h <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107489/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>