Re: End of 2016 update on PyKF5 bindings

LOL, I just posted the review...

On 28 January 2017 at 16:12, Stephen Kelly <[email protected]> wrote:

> Hi Shaheed,
>
> That's great, thanks for keeping us up to date with your progress! Looking
> forward to reviewing it.
>
> Thanks,
>
> Steve.
>
>
> Shaheed Haque wrote:
>
> > Hi Steve,
> >
> > FWIW, I have rebased [1], debugged, bulk tested, and squashed to the
> > point where I can focus on adding to the unit tests for the new
> > functionality [2].
> >
> > Basically, things took longer than expected because when I rebased, I
> > picked up your fix for the regression around reporting errors from the
> > clang compiler. That resulted in errors showing that I had some messed
> > up paths...and when I fixed that, all sort of objects were no longer
> > "int"s (because they were now being understood by clang :-)). The flip
> > side of that is of course that the sip_generator can now correctly
> > handle quite a bit more syntax! I'll be adding some unit tests for
> > that stuff too.
> >
> > You can see the WIP at
> > https://github.com/ShaheedHaque/extra-cmake-modules/tree/KDE_master_
> rebase_with_module_separation
> > (but please don't attempt to merge anything yet).
> >
> > More anon.
> >
> > Thanks, Shaheed
> >
> > [1] To all-but-the-tip of master, that is your
> > 3e6eb0562e5fd3831d66db4720c67cc950b3536c.
> > [2] As well as incorporating your feedback about the second
> > declaration thing and so on.
> >
> > On 22 January 2017 at 15:04, Shaheed Haque <[email protected]> wrote:
> >> I see the point. I'll take another look...
> >>
> >> On 22 January 2017 at 12:25, Stephen Kelly <[email protected]> wrote:
> >>>
> >>> Shaheed Haque wrote:
> >>>
> >>> > Hi Steve,
> >>> >
> >>> > I've tested that this change does not appear to break things. Full
> >>> > details, including the testing, in
> >>> > https://git.reviewboard.kde.org/r/129763/. Kindly review.
> >>>
> >>> Hi Shaheed,
> >>>
> >>> Source files in kcoreaddons are in
> >>>
> >>>  util/kuser.h
> >>>
> >>> for example and then get installed to
> >>>
> >>>  include/KF5/KCoreAddons/kuser.h
> >>>
> >>> for example.
> >>>
> >>> Currently the buildsystem adds both
> >>>
> >>>  include/KF5/KCoreAddons
> >>>
> >>> and
> >>>
> >>>  include/KF5
> >>>
> >>> to the include directories when a consumer users KCoreAddons. That
> means
> >>> that a consumer can write either
> >>>
> >>>  #include <kuser.h>
> >>>
> >>>  (because of the include/KF5/KCoreAddons)
> >>>
> >>> or
> >>>
> >>>  #include <KCoreAddons/kuser.h>
> >>>
> >>>  (because of the include/KF5)
> >>>
> >>>
> >>> I consider the former legacy and I think we should change it to require
> >>> the
> >>> module name for KF6.
> >>>
> >>> So, the Sip file needs to know to process
> >>>
> >>>  util/kuser.h
> >>>
> >>> but it needs to generate
> >>>
> >>>  %TypeHeaderCode
> >>>  #include <KCoreAddons/kuser.h>
> >>>  %End
> >>>
> >>>
> >>> That is why the python interface requires specifying both.
> >>>
> >>> It is true that currently the CMake module/function only requires one
> of
> >>> those, and it is the cmake function that makes the assumption that
> using
> >>> basename is all that is needed. That is only because the CMake API for
> >>> mappings or pairs like that is inconvenient. I do plan on implementing
> >>> it though eventually. Other libraries (outside of KF5) do require
> >>> specifying the directory name like that in includes, so it is a
> >>> requirement anyway.
> >>>
> >>> The assumption that the two are related by exactly a call to basename
> >>> should
> >>> not be in the python code/interface.
> >>>
> >>> Does that make sense?
> >>>
> >>> Thanks,
> >>>
> >>> Steve.
> >>
> >>
>