<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/113406/">http://git.reviewboard.kde.org/r/113406/</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;">IMO the documentation could be improved.
It should mention the assumption about the location of ${actualheader}.
It should also mention the use of ${PROJECT_NAME}.
It needs to mention that it assumes that a variable ${INCLUDE_INSTALL_DIR} is set, and that this is used for installing the generated headers.
I often found that a function/macro started like this, with just a list of arguments, and later on additional options became necessary (the use of PROJECT_NAME and the install dir are candidates here). This is hard to do without having keywords in the arguments which separate the different options.
It would be nice if the signature could be changed to
ecm_generate_headers(CLASSES ClassA...ClassN [MODULE_NAME SomeName] [INSTALL_DIR /some/dir] ...)
using the cmake_parse_arguments() function.
</pre>
<br />
<p>- Alexander Neundorf</p>
<br />
<p>On October 23rd, 2013, 5:38 p.m. UTC, Aleix Pol Gonzalez wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Build System, KDE Frameworks and Stephen Kelly.</div>
<div>By Aleix Pol Gonzalez.</div>
<p style="color: grey;"><i>Updated Oct. 23, 2013, 5:38 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;">Create a macro that will generate the forward headers like we used to, in cmake configure time.
Here's an example of how we'd port KParts to that macro: http://paste.opensuse.org/9880051
After the change, these are the installed headers: http://paste.opensuse.org/90980400</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;">Ported KParts and still everything works.</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>modules/ECMGenerateHeaders.cmake <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113406/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>