cgiboudeaux added inline comments.
INLINE COMMENTS
> FindIcoTool.cmake:1
> +# Copyright 2017 Vincent Pinon <vpi...@kde.org>
> +include(${CMAKE_CURRENT_LIST_DIR}/ECMFindModuleHelpersStub.cmake)
This module doesn't have any license and doesn't provide any doc.
> FindIcoTool.cmake:3
> +include(${CMAKE_CURRENT_LIST_DIR}/ECMFindModuleHelpersStub.cmake)
> +ecm_find_package_version_check(IcoTool)
> +find_program(IcoTool_EXECUTABLE NAMES icotool)
Is this really needed for a simple Find*.cmake module ?
> ECMAddAppIcon.cmake:29
> # target does not have the ``WIN32_EXECUTABLE`` property set.
> -# * The tool png2ico is required. See :find-module:`FindPng2Ico`.
> +# * The tools png2ico or icotool are required.
> +# See :find-module:`FindPng2Ico` or :find-module:`FindIcotool`.
I'm unsure about the grammar here. Only one is needed (and none is required
according to this file).
> ECMAddAppIcon.cmake:30
> +# * The tools png2ico or icotool are required.
> +# See :find-module:`FindPng2Ico` or :find-module:`FindIcotool`.
> # * Supported sizes: 16, 32, 48, 64, 128.
Don't forget to create the matching .rst file in docs/find-module
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D8281
To: vpinon, dfaure, apol, kfunk, #windows
Cc: cgiboudeaux, #frameworks, #build_system
