<table><tr><td style="">kossebau added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D28355">View Revision</a></tr></table><br /><div><div><p>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.</p>

<p>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.<br />
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,</p>

<p>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:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">if(ARGS_UNPARSED_ARGUMENTS)
  message(FATAL_ERROR "Unknown arguments given to ecm_install_configured_file(): \"${ARGS_UNPARSED_ARGUMENTS}\"")
endif()</pre></div>

<p>Otherwise looks good to me as well, no showstoppers on my mind. Not yet tested myself though, only looked at patch code.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28355#inline-166223">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ECMConfiguredInstall.cmake:5</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d">#</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"># Take a list of files, runs configure_file and installs the resultant configured file in the given location.</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d">#</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">"Takes". And: configured "files", given "list of files"?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28355#inline-166242">View Inline</a><span style="color: #4b4d51; font-weight: bold;">CMakeLists.txt:2</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">project(ECMConfiguredInstallTest)
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">cmake_minimum_required(VERSION 3.5)
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../modules)
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">cmake_minimum_required should be first line always, for consistent pattern.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R240 Extra CMake Modules</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28355">https://phabricator.kde.org/D28355</a></div></div><br /><div><strong>To: </strong>davidedmundson, Build System<br /><strong>Cc: </strong>apol, kossebau, pino, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, michaelh, ngraham, bruns<br /></div>