D28355: Introduce function ecm_install_configured_file

[Friedrich W. H. Kossebau]

kossebau added a comment.


  A bit unsure if the arg name "TEMPLATES" is good, or if perhaps should be renamed to "INPUT". Just mentioning, not preferring one over the other. So far have not found existing samples to take as lead for consistent argument naming.
  
  For completeness, would be good to test for presence of ARGS_DESTINATION, given this is a required arg, and do an error report, instead of falling flat internally on the install() command.
  ARGS_TEMPLATES should be fine to be empty, no need to error out on that, might happen if input on caller side is based on a var which gets filled conditionally and might eventually end with empty list,
  
  And while talking checking input, I recently learned about the foillowing handling, and think it provides users of the macros some service in case of typos or accidental misuse, so propose to also add here:
  
    if(ARGS_UNPARSED_ARGUMENTS)
      message(FATAL_ERROR "Unknown arguments given to ecm_install_configured_file(): \"${ARGS_UNPARSED_ARGUMENTS}\"")
    endif()
  
  Otherwise looks good to me as well, no showstoppers on my mind. Not yet tested myself though, only looked at patch code.

INLINE COMMENTS

> ECMConfiguredInstall.cmake:5
> +#
> +# Take a list of files, runs configure_file and installs the resultant configured file in the given location.
> +#

"Takes". And: configured "files", given "list of files"?

> CMakeLists.txt:2
> +project(ECMConfiguredInstallTest)
> +cmake_minimum_required(VERSION 3.5)
> +set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../modules)

cmake_minimum_required should be first line always, for consistent pattern.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidedmundson, #build_system
Cc: apol, kossebau, pino, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200423/53285b9b/attachment.html>