D8281: Allow to use IcoTool for Windows icons

Christophe Giboudeaux noreply at phabricator.kde.org
Tue Nov 7 12:42:20 UTC 2017


cgiboudeaux added inline comments.

INLINE COMMENTS

> FindIcoTool.cmake:1
> +# Copyright 2017 Vincent Pinon <vpinon at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20171107/1312f632/attachment.html>


More information about the Kde-buildsystem mailing list