kossebau added a comment.
Quick review while I had some spare minutes, to keep things going. INLINE COMMENTS > ECMConfiguredInstall.cmake:5 > +# > +# Take a list of files, runs configure_file and installs it in the given > location. > +# Perhaps "install" -> "install the result" > ECMConfiguredInstall.cmake:9 > +# :: > +# ecm_install_configured_files(TEMPLATES <file> [<file2> [...]] > DESTINATION <INSTALL_DIRECTORY> [COPYONLY] [ESCAPE_QUOTES] [@ONLY] [COMPONENT > <component>]) > +# Might be nicer to have each argument on an own line, like done in the (most?) other docs, like this: # ecm_install_configured_files( # TEMPLATES <file> [<file2> [...]] # DESTINATION <INSTALL_DIRECTORY> # [COPYONLY] # [ESCAPE_QUOTES] # [@ONLY] # [COMPONENT <component>] # ) > ECMConfiguredInstall.cmake:18 > +# This wil install the file as foo.txt with any cmake variable replacements > made into the data directory. > + > +# Copyright 2020 David Edmundson <davidedmund...@kde.org> `# Since 5.69/70.0.` > ECMConfiguredInstall.cmake:56 > + > + string(REGEX REPLACE ".in$" "" _name ${_name}) > + Being a regexp, the "." in ".in$" might need escaping. > ECMConfiguredInstall.cmake:62 > + if (ARGS_COPY_ONLY) > + string(APPEND _configure_args COPY_ONLY) > + endif() These strings (besides the last obviously) should get added with whitespace suffix, to handle the case where multiple are added, no? Not yet got to test/run things, just guessing by reading code. > check_tree.cmake.in:4-11 > +execute_process(COMMAND ${CMAKE_COMMAND} -E compare_files > ${ACTUAL}/test1/configured.txt > + ${EXPECTED}/configured.txt > + RESULT_VARIABLE test1_result > +) > + > +If (NOT test1_result EQUAL 0) > + message(FATAL_ERROR "Configured files differ!") This could become a macro/function perhaps, instead of repeating the same logic 4 times. Actually one that should get moved to tests/test_helpers.cmake later, as I remember other places which also check generated files against file samples. But can also be done as follow-up by someone (tm). REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D28355 To: davidedmundson Cc: kossebau, pino, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, GB_2, bencreasy, michaelh, ngraham, bruns