-----------------------------------------------------------
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
> 
>

_______________________________________________
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Reply via email to