<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/126939/">https://git.reviewboard.kde.org/r/126939/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 1st, 2016, 6:15 p.m. UTC, <b>Holger Kaelberer</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;">We default to "auto", which means in most cases "submodule", because something like a "system"-provided (read: packaged) qml-box2d does not yet exist in most distros. Defaulting to system would imho be fine if most systems would provide the dependency.
It's not uncommon that git repos use submodules for dependencies that are populated at build-time. Or do we break some KDE policy here?
Cf. also https://mail.kde.org/pipermail/gcompris-devel/2015-October/004375.html</pre>
</blockquote>
<p>On February 1st, 2016, 6:27 p.m. UTC, <b>Nicolás Alvarez</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;">You break most distros' policies (they enforce it by having build servers have no access to the network) and most user expectations (downloading at code at compile time is not something I want and I will instantly Ctrl-C if I catch it).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It's better to say "dependency not found", and let the user choose between installing it from the distro if available, compiling it and installing it by hand, or setting <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QML_BOX2D_MODULE=submodule</code>.</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;">The idea was that for packaging the maintainer is supposed to provide the external dependency and specify QML_BOX2D_MODULE=system when building GCompris. I guess Bruno has done that for the SUSE build. That is not always possible?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Regarding the user expectations: The idea behind the "auto" default was to make handling convenient especially for developers, especially for the many newcomers we have in GCompris that start contributing to open source via GSOC/SOK/GCI. So IIUC this now conflicts with other common practices.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Doing git submodule init && git submodule udpate in configure/cmake step might admittedly be too much of convenience :-) In the autotools world this is often done in the autogen/autoreconf step.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Agreed that we should error-out in case the dependency is not there instead of automatically deactivating depending stuff, because this might not be what the dev wants.</p></pre>
<br />
<p>- Holger</p>
<br />
<p>On January 31st, 2016, 3:32 p.m. UTC, Sune Vuorela 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 Build System and GCompris.</div>
<div>By Sune Vuorela.</div>
<p style="color: grey;"><i>Updated Jan. 31, 2016, 3:32 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
gcompris
</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 was negatively surprised to having a default build doing network traffic. This is an attempt to make the default less surprising</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">a default setup now builds without networking</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">(371b6d3)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126939/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>