<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/129760/">https://git.reviewboard.kde.org/r/129760/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 5th, 2017, 10:56 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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I've added the change to the unit test. It already passes, so it's not clear to me what else is needed from this review request.</p></pre>
</blockquote>
<p>On January 5th, 2017, 11:06 p.m. UTC, <b>Shaheed Haque</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;">What version of SIP compiler and C++ compiler are you using? I'm on:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">$ sip -V
4.18.1
$ g++ --version
g++ (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In any case, unless the change breaks thing for you, I would prefer to merge the complete change as it is definitely needed here.</p></pre>
</blockquote>
<p>On January 5th, 2017, 11:11 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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">$ sip -V
4.17
$ g++ --version
g++ (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">$ cat /etc/issue
Ubuntu 16.04.1 LTS \n \l</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Can you ensure that when you test this that you are using the clean master branch of ECM and the master branch of KGuiAddons? If you have </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">https://git.reviewboard.kde.org/r/129759/</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">applied or any other patches, that could affect what you get when you run the test.</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;">As per my reviewboard markings, this change depends on '759. The point is that without '759, the SIP compiler bails out before this issue can be seen. So the real question is why/how you were able to manage without '759...I would guess that the SIP compiler version delta explains this.</p></pre>
<br />
<p>- Shaheed</p>
<br />
<p>On January 3rd, 2017, 12:47 p.m. UTC, Shaheed Haque 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.</div>
<div>By Shaheed Haque.</div>
<p style="color: grey;"><i>Updated Jan. 3, 2017, 12:47 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kguiaddons
</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;">This depends on a fix in extra-cmake-module to actually emit the
default value for the flags parameter.</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;">Without this change, once the code the depends-on review is committed, kguiaddons will fail to compile as follows:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In file included from /home/.../PyKF5/KGuiAddons/unifiedKGuiAddons.cpp:1:0:
/home/.../PyKF5/KGuiAddons/sipKGuiAddonsKFontUtils.cpp: In function ‘PyObject<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> meth_KFontUtils_adaptFontSize(PyObject</em>, PyObject*)’:
/home/...d/PyKF5/KGuiAddons/sipKGuiAddonsKFontUtils.cpp:30:50: error: ‘NoFlags’ was not declared in this scope
KFontUtils::AdaptFontSizeOptions a6def = NoFlags;</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">With the change, it compiles and the tests run:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">$ ctest
Test project /home/srhaque/kdebuild/kguiaddons
Start 1: appstreamtest
1/6 Test #1: appstreamtest .................... Passed 0.02 sec
Start 2: Py3Test
2/6 Test #2: Py3Test .......................... Passed 0.13 sec
Start 3: Py2Test
3/6 Test #3: Py2Test .......................... Passed 0.11 sec
Start 4: kwordwraptest
4/6 Test #4: kwordwraptest .................... Passed 0.16 sec
Start 5: kcolorutilstest
5/6 Test #5: kcolorutilstest .................. Passed 0.53 sec
Start 6: kiconutilstest
6/6 Test #6: kiconutilstest ................... Passed 0.08 sec</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">100% tests passed, 0 tests failed out of 6</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Total Test time (real) = 1.03 sec</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>autotests/pythontest.py <span style="color: grey">(c93e75397f87ba4a84f834dd248d671614ac89dd)</span></li>
<li>cmake/rules_PyKF5.py <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(63e7598d13fa1c4d9d67e2f99edcc13e0269920e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129760/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>