<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/104537/">http://git.reviewboard.kde.org/r/104537/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 13th, 2012, 9:27 p.m., <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/104537/diff/2/?file=56290#file56290line42" style="color: black; font-weight: bold; text-decoration: underline;">projectbuilders/makebuilder/imakebuilder.h</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">42</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">please don't inline this (it's virtual anyways), put it into the .cpp file</pre>
</blockquote>
<p>On April 14th, 2012, 3:17 a.m., <b>Alexandre Courbot</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;">There is no cpp file for IMakeBuilder yet, but ok.</pre>
</blockquote>
<p>On April 14th, 2012, 7:54 a.m., <b>Andreas Pakulat</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;">If you're adding non-pure-virtual functions to IMakeBuilder you're doing the wrong thing already. The interface should not have any such functions, its just an API definition.</pre>
</blockquote>
<p>On April 14th, 2012, 9:23 p.m., <b>Milian Wolff</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;">true, apaku has a point. leave the default-implementation to the implemenation, not to the interface.</pre>
</blockquote>
<p>On April 15th, 2012, 6:44 a.m., <b>Alexandre Courbot</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;">How about IProjectBuilder, which also has implemented methods for e.g. configure()? I am fine with slipping these into the implementation, but this means code duplication if another implementation gets in. We are using C++, not Java - is there any *practical* reason to take interfaces so literally?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Giving it a second thought, keeping executeMakeTarget() purely virtual would not break the ABI, which is good. I will only extend IMakeBuilder with a multi-target + variables variant of it and it should fulfill the purpose while keeping things purely virtual.</pre>
<br />
<p>- Alexandre</p>
<br />
<p>On April 11th, 2012, 2:13 a.m., Alexandre Courbot 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 Alexandre Courbot.</div>
<p style="color: grey;"><i>Updated April 11, 2012, 2:13 a.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 change augments the IMakeBuilder interface and MakeBuilder class to let them support the following:
1) Let make be run with multiple targets to build in one run
2) Pass build variables as a QMap of (variable, value) pairs that are also passed to make's command line.
E.g. this change now makes it possible for the make builder to perform make invokations that look like the following (example taken from an actual Linux kernel build):
$ make ARCH=arm CROSS_COMPILE=/usr/bin/arm-elf- vmlinux modules
API compatibility is not broken, but ABI is as the former virtual method of IMakeBuilder is now an inline function.</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;">Ensured API remained compatible and prior API behaved identically, tested build variables with the kdev-kernel plugin that uses them.</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>projectbuilders/makebuilder/imakebuilder.h <span style="color: grey">(56735425d78551883f109e942145eba2aa982687)</span></li>
<li>projectbuilders/makebuilder/makebuilder.h <span style="color: grey">(34881c6eaee775b6b8b53959dfcf825732e806da)</span></li>
<li>projectbuilders/makebuilder/makebuilder.cpp <span style="color: grey">(6c6905db30f469958f4a0048826febea29bad15a)</span></li>
<li>projectbuilders/makebuilder/makejob.h <span style="color: grey">(19032fdf371da793d52b3457e5aa78a6b8458150)</span></li>
<li>projectbuilders/makebuilder/makejob.cpp <span style="color: grey">(ad5636dbfdadf3ae18ad1cc5b8dff078dd34cd42)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104537/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>