D17547: Bring FindFontconfig.cmake up to ECM standards

Christophe Giboudeaux noreply at phabricator.kde.org
Thu Dec 13 11:51:09 GMT 2018


cgiboudeaux added inline comments.

INLINE COMMENTS

> FindFontconfig.cmake:10
> +#     True if Fontconfig is available
> +# ``FONTCONFIG_INCLUDE_DIR``
> +#     The include directory to use for the Fontconfig headers

shall be FONTCONFIG_INCLUDE_DIRS

(https://cmake.org/cmake/help/latest/manual/cmake-developer.7.html#standard-variable-names)

This link also suggests the variable names shall be exact-case FONTCONFIG_ → Fontconfig_

> FindFontconfig.cmake:50-53
>  if (FONTCONFIG_LIBRARIES AND FONTCONFIG_INCLUDE_DIR)
>  
>    # in cache already
>    set(FONTCONFIG_FOUND TRUE)

Remove this condition. This is not recommended anymore

> FindFontconfig.cmake:57
>  
>    if (NOT WIN32)
>      # use pkg-config to get the directories and then use these values

Delete this if() / endif(). Not needed anymore since ages.

> FindFontconfig.cmake:78
>    )
>  
>    include(FindPackageHandleStandardArgs)

There's no version check, does it matter?

> FindFontconfig.cmake:84
>  
>  endif (FONTCONFIG_LIBRARIES AND FONTCONFIG_INCLUDE_DIR)
>  

endif()

> FindFontconfig.cmake:98
> +  URL "https://www.fontconfig.org/"
> +  DESCRIPTION "Fontconfig is a library for configuring and customizing font access."
> +)

Remove the dot, feature_summary() adds a comma after the description

REPOSITORY
  R108 KWin

REVISION DETAIL
  https://phabricator.kde.org/D17547

To: vkrause, #build_system
Cc: cgiboudeaux, kwin, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20181213/4b613be6/attachment.html>


More information about the Kde-buildsystem mailing list