Review Request 122317: Extend ecm_generate_headers macro to also support CamelCase.h headers
Alex Merry
alex.merry at kde.org
Sun Feb 15 17:46:51 UTC 2015
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122317/#review76062
-----------------------------------------------------------
Ship it!
Seems generally sensible. Just one thing that would make it nicer.
modules/ECMGenerateHeaders.cmake
<https://git.reviewboard.kde.org/r/122317/#comment52489>
Rather than artificially add yet another place to change when we introduce new variants, why not just make this part of the if/else statement in lines 148-152 below?
- Alex Merry
On Feb. 14, 2015, 4:11 p.m., Friedrich W. H. Kossebau wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122317/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2015, 4:11 p.m.)
>
>
> Review request for Build System, Extra Cmake Modules and Alex Merry.
>
>
> Repository: extra-cmake-modules
>
>
> Description
> -------
>
> There are at least two projects I know which have CamelCase.h headers, Calligra and KDChart. Still it would be nice to also allow pure CamelCase headers (suffix less) for usage by lib-using developers. Just, currently the macro ecm_generate_headers assumes the original headers to be lowercase. So it cannot be used for the mentioned projects.
> Worse, ecm_generate_headers also has a very nice additional feature, having copies of the original headers in the build dir created with the same layout as there will be after installation, so e.g. code of demos and tests can be written as if it is 3rd-party outside the project.
>
> Attached patch extends ecm_generate_headers to support an optional parameter `ORIGINAL` (perhaps a better term could be found), by which the casing of the original headers can be specified. Currently `CAMELCASE` and `LOWERCASE` are supported. But perhaps one day someone could extend it for underscore-casing :). The parameter defaults to `LOWERCASE` and thus should be backwards portable.
>
> Tested also with prefixes.
>
> Not sure the assumption that the original prefix has the same casing as the original headers, I remember to have seen code where that is not consistent. But that step to implement is left for those persons who really need it ;)
>
> Update:
> See also discussion at https://git.reviewboard.kde.org/r/122193/ where so far opinions are positive on this one.
>
>
> Diffs
> -----
>
> modules/ECMGenerateHeaders.cmake bac5086
> tests/ECMGenerateHeadersTest/CamelCaseHeadTest1.h PRE-CREATION
> tests/ECMGenerateHeadersTest/CamelCaseHeadTest2.h PRE-CREATION
> tests/ECMGenerateHeadersTest/run_test.cmake.config 0a2425f
>
> Diff: https://git.reviewboard.kde.org/r/122317/diff/
>
>
> Testing
> -------
>
> Manually and in latest update also with 2 unit tests (hm, not sure if those tests should cover more combinations with other parameters).
>
>
> Thanks,
>
> Friedrich W. H. Kossebau
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20150215/5f91a7db/attachment.html>
More information about the Kde-buildsystem
mailing list