shaheed added inline comments.

INLINE COMMENTS

> lbeltrame wrote in CMakeLists.txt:49
> Don't use `find_package(FOO REQUIRED)`. Use `find_package(FOO)` then 
> `set_package_properties(FOO TYPE REQUIRED...`. There are many examples in KDE 
> git you can use. You will need to include FeatureSummary for this to work 
> properly.
> 
> Why do I say this? Because now CMake will fail *immediately* at the first 
> error. Otherwise, it will collect and print all missing packages after 
> configuration. It is much easier on the packagers.

Good point. I even helped update the Techbase docs for this, but then forgot to 
implement it :-).

> lbeltrame wrote in CMakeLists.txt:53
> Any specific reason for this SIP version?

Yes, this contains needed patches. Though it turns out that even this will not 
be sufficient because I submitted further patches for more issues. 
Unfortunately, upstream decided to implement some of those patches using 
Python3 only and I'm still stuck on Python2 because of the CLang dependency.

CLang Python3 support was shipped yesterday.

When it is available, a newer version of SIP that contains the support for 
scoped enums as per 
https://www.riverbankcomputing.com/pipermail/pyqt/2017-August/039494.html will 
need to be integrated (the core logic here may or may not need to change 
slightly).

> lbeltrame wrote in FindClang.cmake:52
> This won't work on all distros. Can you either:
> 
> a. Take inspiration from KDevelop's FindClang (which works already nicely)
> b. Use upstream Clang module to find it and its version?
> 
> The same comment goes for FindLibClang.

I'm well aware of the issue. Unfortunately, upstream CLang was not available to 
me, and the KDevelop one is not sufficiently complete to do what I need.

Hence this would need addressing in due course.

> lbeltrame wrote in HOWTO:12
> Before distro-specific instructions, put a list of all the dependencies with 
> their upstream names. This ensures that packagers of non Ubuntu/Debian 
> distros can also look them up.

Yup. I'd likely need a bit of help at that point, but I would hope that using 
KDE's CI system would be a good starting point.

Another issue that would need to be addressed is the fact that the newer 
versions of SIP will only support Python3 as noted above.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7736

To: shaheed, lbeltrame
Cc: #frameworks, #build_system

Reply via email to