Review Request 113406: Add a macro to automatically generate forward headers

Alexander Neundorf neundorf at kde.org
Wed Oct 23 20:21:32 UTC 2013


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113406/#review42250
-----------------------------------------------------------


IMO the documentation could be improved.
It should mention the assumption about the location of ${actualheader}.
It should also mention the use of ${PROJECT_NAME}.
It needs to mention that it assumes that a variable ${INCLUDE_INSTALL_DIR} is set, and that this is used for installing the generated headers.

I often found that a function/macro started like this, with just a list of arguments, and later on additional options became necessary (the use of PROJECT_NAME and the install dir are candidates here). This is hard to do without having keywords in the arguments which separate the different options.
It would be nice if the signature could be changed to
ecm_generate_headers(CLASSES ClassA...ClassN [MODULE_NAME SomeName] [INSTALL_DIR /some/dir] ...)
using the cmake_parse_arguments() function.


- Alexander Neundorf


On Oct. 23, 2013, 5:38 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113406/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 5:38 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks and Stephen Kelly.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> -------
> 
> Create a macro that will generate the forward headers like we used to, in cmake configure time.
> 
> Here's an example of how we'd port KParts to that macro: http://paste.opensuse.org/9880051
> After the change, these are the installed headers: http://paste.opensuse.org/90980400
> 
> 
> Diffs
> -----
> 
>   modules/ECMGenerateHeaders.cmake PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/113406/diff/
> 
> 
> Testing
> -------
> 
> Ported KParts and still everything works.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20131023/36415161/attachment.html>


More information about the Kde-buildsystem mailing list