----------------------------------------------------------- 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 > >
_______________________________________________ Kde-buildsystem mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-buildsystem
