D9624: Install parser headers and cmake config files to support client packages

Kevin Funk noreply at phabricator.kde.org
Tue Jan 9 13:01:03 UTC 2018


kfunk requested changes to this revision.
kfunk added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> CMakeLists.txt:3
>  
>  project(php)
>  

`project(kdev-php VERSION 5.1.2)` please. And then just `PROJECT_VERSION` & friends below.

See: https://cmake.org/cmake/help/v3.0/command/project.html

> CMakeLists.txt:17
> +ecm_setup_version(
> +    5.1.2
> +    VARIABLE_PREFIX KDEVPHP

We'll have a hard time updating the kdev-php version via scripts then.

> CMakeLists.txt:43
>      ${KDEVPGQT_INCLUDE_DIR}
> +    ${CMAKE_SOURCE_DIR}
>  )

Why that?

I guess that should be `CMAKE_CURRENT_BINARY_DIR`?

> CMakeLists.txt:26
>  
> +target_include_directories(kdevphpparser PUBLIC
> +  $<INSTALL_INTERFACE:${KDEVPHP_PRIVATE_INCLUDE_DIR}/parser>

Can be merged in one single `target_include_directories(...)` call.

> pprkut wrote in CMakeLists.txt:49
> Seems a bit odd to me to install the grammar file. Afaics phpparser.h is generated from it and should have everything one needs, no?

+1

REPOSITORY
  R52 KDevelop: PHP Support

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

To: habacker, pprkut, kfunk, mtijink
Cc: brauch, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20180109/a6516b27/attachment.html>


More information about the KDevelop-devel mailing list