<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/123477/">https://git.reviewboard.kde.org/r/123477/</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 23rd, 2015, 3:35 p.m. CEST, <b>Aleix Pol Gonzalez</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;">And yes, this is required as long as NetworkManager headers are included by NetworkManagerQt headers.</p></pre>
</blockquote>
<p>On April 23rd, 2015, 3:40 p.m. CEST, <b>Jan Grulich</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 just found out that installing FindNetworkManager.cmake into CMake modules is not enough, because it still cannot find it. I guess this would have to be done according to other FindFoo.cmake modules from extra-cmake-modules, right?</p></pre>
</blockquote>
<p>On April 23rd, 2015, 4:24 p.m. CEST, <b>Aleix Pol Gonzalez</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;">That would certainly help, but then it's not very nice putting a cmake file into another project's directory.
I would suggest to either:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Contribute FindNetworkManager.cmake to extra-cmake-modules</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Install it within the NetworkManagerQt cmake directory, together with NetworkManagerQtConfig.cmake.</li>
</ul></pre>
</blockquote>
<p>On April 23rd, 2015, 4:59 p.m. CEST, <b>Jan Grulich</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 actually didn't mean that I should install it to ../ECM/modules, but whether using find_package_handle_standard_args() and other macros, which are used in all FindFoo.cmake modules from e-c-m, are required to make this work.</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;">Not required per se, but recommended. There's many things to be defined on a proper cmake finder.
OTOH, this finder seems to be working already.</p></pre>
<br />
<p>- Aleix</p>
<br />
<p>On April 23rd, 2015, 3:01 p.m. CEST, Jan Grulich 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 Frameworks, David Faure, Daniel Vrátil, and Lamarque Souza.</div>
<div>By Jan Grulich.</div>
<p style="color: grey;"><i>Updated April 23, 2015, 3:01 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
networkmanager-qt
</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;">I'm not 100% sure how this should work and I couldn't find another framework doing the same, but Daniel Vrátil pointed me out that NetworkManagerQt should mention in KF5NetworkManagerQtConfig.cmake file that it requires NetworkManager as dependency. Given this, we also need to install FindNetworkManager.cmake into CMake modules so NetworkManager can be found by find_dependency() macro, or FindNetworkManager.cmake can go into extra-cmake-modules.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If this is how it should be done, then similar patch would be needed for ModemManagerQt.</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">(c9e3274)</span></li>
<li>KF5NetworkManagerQtConfig.cmake.in <span style="color: grey">(cdabe8e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123477/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>