Review Request 119798: Generating PkgConfig files from ECM

[Alex Merry]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119798/#review65841
-----------------------------------------------------------


Sorry to still be nitpicking documentation, but I view it as one of the most important parts of the code.


modules/ECMGeneratePkgConfigFile.cmake
<https://git.reviewboard.kde.org/r/119798/#comment45997>

    Not really true any more (while the variable is created, it is only maybe done by the function, and the default value depends on the first call of the function with the INSTALL flag). So I'd just discard this sentence.



modules/ECMGeneratePkgConfigFile.cmake
<https://git.reviewboard.kde.org/r/119798/#comment45999>

    You should explicitly say this is the name that projects will use when querying pkg-config



modules/ECMGeneratePkgConfigFile.cmake
<https://git.reviewboard.kde.org/r/119798/#comment45998>

    Talking about the "Name:" field is not very useful for people not very familiar with pkg-config. It would be better to say that it's the human-readable name in pkg-config.
    
    I wonder if that should even be a separate argument.



modules/ECMGeneratePkgConfigFile.cmake
<https://git.reviewboard.kde.org/r/119798/#comment46000>

    space (receivethe)



modules/ECMGeneratePkgConfigFile.cmake
<https://git.reviewboard.kde.org/r/119798/#comment46001>

    missing y (subdirector)


- Alex Merry


On Sept. 3, 2014, 10:53 a.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119798/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2014, 10:53 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks and Harald Sitter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> -------
> 
> So we decided we wanted those .pc files, so I created a small script that generates one, I haven't used pc in the past, so feedback is welcome.
> 
> 
> Diffs
> -----
> 
>   modules/ECMGeneratePkgConfigFile.cmake PRE-CREATION 
>   tests/CMakeLists.txt 65de038 
>   tests/ECMGeneratePkgConfigFile/CMakeLists.txt PRE-CREATION 
>   tests/ECMGeneratePkgConfigFile/KF5CoreAddons.pc PRE-CREATION 
>   tests/ECMGeneratePkgConfigFile/run_test.cmake.config PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119798/diff/
> 
> 
> Testing
> -------
> 
> I added it in KCoreAddons, this is the patch:
> diff --git src/lib/CMakeLists.txt src/lib/CMakeLists.txt
> index 26eb5a1..3a07d1c 100644
> --- src/lib/CMakeLists.txt
> +++ src/lib/CMakeLists.txt
> @@ -188,4 +188,6 @@ install(FILES
>  
>  include(ECMGeneratePriFile)
>  ecm_generate_pri_file(BASE_NAME KCoreAddons LIB_NAME KF5CoreAddons DEPS "core" FILENAME_VAR PRI_FILENAME INCLUDE_INSTALL_DIR ${KF5_INCLUDE_INSTALL_DIR}/KCoreAddons)
> +ecm_generate_pkgconfig_file(BASE_NAME KCoreAddons LIB_NAME KF5CoreAddons DEPS Qt5Core INCLUDE_INSTALL_DIR ${KF5_INCLUDE_INSTALL_DIR}/KCoreAddons INSTALL)
>  install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
> 
> This is the result, on my system:
> 
> Name: KF5CoreAddons
> Version: 5.1.0
> Libs: -L/home/kde-devel/kde5/lib64 -l/home/kde-devel/kde5/lib64
> Cflags: -I/home/kde-devel/kde5/include/KF5/KCoreAddons 
> Requires: Qt5Core
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140905/19039b0f/attachment.html>