<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/112928/">http://git.reviewboard.kde.org/r/112928/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 26th, 2013, 8:27 p.m. UTC, <b>Alexander Neundorf</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/112928/diff/5/?file=192796#file192796line22" style="color: black; font-weight: bold; text-decoration: underline;">template/CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<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">22</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c"># find_package(KCoreAddons ${KF5_VERSION} REQUIRED)</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;">The idea here was that you can simply list all required KF5 frameworks in one find_package() call:
find_package(KF5 COMPONENTS CMake Compiler InstallDirs KCoreAddons Solid ....)
When not doing this, you can also use the longer include() syntax instead of the find_package(KF5) syntax:
include(KDECMakeSettings)
include(KDECompilerSettings)
include(KDEInstallDirs)
find_package(KCoreAddons)
</pre>
</blockquote>
<p>On September 26th, 2013, 8:49 p.m. UTC, <b>Stephen Kelly</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;">What would make sense to me is this:
As a downstream KDE application
find_package(ECM 0.0.9 REQUIRED KDECMake KDECompiler KDEInstallDirs)
find_package(KF5 5.0.0 REQUIRED KCoreAddons Solid)
As a downstream which is not a KDE application:
find_package(KF5 5.0.0 REQUIRED KCoreAddons Solid)
In the KF5 tier1 buildsystems:
find_package(ECM 0.0.9 REQUIRED KDECMake KDECompiler KDEInstallDirs)
In the KF5 tier>1 buildsystems:
find_package(ECM 0.0.9 REQUIRED KDECMake KDECompiler KDEInstallDirs)
find_package(KF5 5.0.0 REQUIRED KCoreAddons Solid)
That way, when we're building KF5 tier1, we're not finding KF5. We're finding and using ECM.
When we're building KF5 tier2, we're finding out tier1 deps and we're finding and using ECM.
etc.
That maps to reality. It's a bit unfortunate that find_package(KF5) has to be a FindKF5 in ECM, but that's ok IMO.
Thanks,
Steve.</pre>
</blockquote>
<p>On September 27th, 2013, 2:20 p.m. UTC, <b>Aurélien Gâteau</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;">Alexander: I am still not sure what the arguments after REQUIRED are, I just cargo-culted them. Are you saying this is just a way to avoid adding include() lines? If so, how comes the arguments after REQUIRED are not exactly the same as the one you used in your include() lines?
Steve: this sounds good to me, but would be tricky to keep readable if we fit all in one template file. Maybe it would be better to create different templates, like templates/tier1, templates/tiern, templates/kde-app, templates/qt-app? I am worried about the templates bit-rotting though.</pre>
</blockquote>
<p>On September 27th, 2013, 2:50 p.m. UTC, <b>Stephen Kelly</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;">My comment was more about what is maintained in the repos and what I find readable and what I think maps to reality. My comment was not spefically about what a template-based generator would generate. After being generated from a template the code will have to be changed and maintained anyway.
I'd put a comment in the single template that you have, not create multiple templates. That would only have people wondering what to use. Note though that what I said would make sense to me is not the current state.</pre>
</blockquote>
<p>On September 27th, 2013, 3:06 p.m. UTC, <b>Aurélien Gâteau</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;">Oh OK. Actually, I would not put instructions for application in there, just tier1 and tier N+1. Application instructions should be kept somewhere else IMO.</pre>
</blockquote>
<p>On September 27th, 2013, 8:26 p.m. UTC, <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;">find_package(ECM 0.0.9 REQUIRED KDECMake KDECompiler KDEInstallDirs)
find_package(KF5 5.0.0 REQUIRED KCoreAddons Solid)
I see what you mean.
Still, when starting that way, i.e. making KDECMake etc. components of ECM, then all other find-modules of ECM could in the same way be considered components of ECM.
As I see it, they belong to KDE, at least logically, so they are part of KDE frameworks.</pre>
</blockquote>
<p>On September 27th, 2013, 8:29 p.m. UTC, <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;">Aurelien: the component keywords are evaluated by FindKF5.cmake, which translates them internally into the real filenames. If you simply use include(), you have to use the real filenames.
They differ just so it is less to write/looks prettier:
find_package(KF5 COMPONENTS CMake Compiler InstallDirs Solid ...)
instead of
find_package(KF5 COMPONENTS KDECMakeSettings KDECompilerSettings KDEInstallDirs Solid ...)
The components are anyway already "namespaced" by being part of the find_package() call for KF5.
</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;">Stephen: and what I forgot: technically, "KDECMake" would be a component of ECM, ECMConfig.cmake would have to contain code for these KDE-files. I did not want that.
As it is, all the KDE-specific stuff is nicely separated in the kde-modules/ folder.
And yes, a separate "these are the cmake files specific for KDE and nothing else" package would have been cleaner.</pre>
<br />
<p>- Alexander</p>
<br />
<p>On September 27th, 2013, 3:05 p.m. UTC, Aurélien Gâteau 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 KDE Frameworks, Kevin Ottens, Alexander Neundorf, and Stephen Kelly.</div>
<div>By Aurélien Gâteau.</div>
<p style="color: grey;"><i>Updated Sept. 27, 2013, 3:05 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 adds a template/ dir which contains example CMakeLists.txt and FooBarConfig.cmake.in files, based on what exists in current frameworks.
</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>template/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>template/FooBarConfig.cmake.in <span style="color: grey">(PRE-CREATION)</span></li>
<li>template/autotests/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>template/setup.sh <span style="color: grey">(PRE-CREATION)</span></li>
<li>template/src/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>template/src/myclass.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>template/src/myclass.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>template/tests/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112928/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>