Review Request 129760: Fixup handling of KFontUtils::adaptFontSize's flags' default value.

[Shaheed Haque]

Thanks for the update, I'll take a look!

On 13 January 2017 at 19:15, Stephen Kelly <steveire at gmail.com> wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129760/
>
> On January 5th, 2017, 10:56 p.m. UTC, *Stephen Kelly* wrote:
>
> 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.
>
> On January 5th, 2017, 11:06 p.m. UTC, *Shaheed Haque* wrote:
>
> What version of SIP compiler and C++ compiler are you using? I'm on:
>
> $ 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.
>
> In any case, unless the change breaks thing for you, I would prefer to merge the complete change as it is definitely needed here.
>
> On January 5th, 2017, 11:11 p.m. UTC, *Stephen Kelly* wrote:
>
> I have
>
> $ 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.
>
> $ cat /etc/issue
> Ubuntu 16.04.1 LTS \n \l
>
> 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
>
> https://git.reviewboard.kde.org/r/129759/
>
> applied or any other patches, that could affect what you get when you run the test.
>
> On January 6th, 2017, 8:54 a.m. UTC, *Shaheed Haque* wrote:
>
> 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.
>
> Please see https://bugs.kde.org/show_bug.cgi?id=374801 and the commits associated with it.
>
> In particular: your compiles were failing, but bindings were being created anyway, which mostly worked for you with the exception of this enum issue. This enum issue failed because it relies on being able to parse the QFlags properly. Your compilations were failing because you are using Qt 5.7 or later which requires -std=c++11 or later. That was not being added to the flags when parsing the headers with libclang. ECM commit 8aa6843404f9c6faef66cb9c76358158eafc1af1 fixed the parse failure, and ed1b9ce2bb2a2e51410e0a1754a72c110010a6a0 fixed the logging bug.
>
> Please verify that you can build kguiaddons master with ECM master.
>
>
> - Stephen
>
> On January 3rd, 2017, 12:47 p.m. UTC, Shaheed Haque wrote:
> Review request for KDE Frameworks.
> By Shaheed Haque.
>
> *Updated Jan. 3, 2017, 12:47 p.m.*
> *Repository: * kguiaddons
> Description
>
> This depends on a fix in extra-cmake-module to actually emit the
> default value for the flags parameter.
>
> Testing
>
> Without this change, once the code the depends-on review is committed, kguiaddons will fail to compile as follows:
>
> In file included from /home/.../PyKF5/KGuiAddons/unifiedKGuiAddons.cpp:1:0:
> /home/.../PyKF5/KGuiAddons/sipKGuiAddonsKFontUtils.cpp: In function ‘PyObject* meth_KFontUtils_adaptFontSize(PyObject*, PyObject*)’:
> /home/...d/PyKF5/KGuiAddons/sipKGuiAddonsKFontUtils.cpp:30:50: error: ‘NoFlags’ was not declared in this scope
>          KFontUtils::AdaptFontSizeOptions a6def = NoFlags;
>
> With the change, it compiles and the tests run:
>
> $ 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
>
> 100% tests passed, 0 tests failed out of 6
>
> Total Test time (real) =   1.03 sec
>
> Diffs
>
>    - autotests/pythontest.py (c93e75397f87ba4a84f834dd248d671614ac89dd)
>    - cmake/rules_PyKF5.py (PRE-CREATION)
>    - src/CMakeLists.txt (63e7598d13fa1c4d9d67e2f99edcc13e0269920e)
>
> View Diff <https://git.reviewboard.kde.org/r/129760/diff/>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170114/8c6718fe/attachment-0001.html>