<table><tr><td style="">kossebau added inline comments.
</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/D22790">View Revision</a></tr></table><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/D22790#inline-128721">View Inline</a><span style="color: #4b4d51; font-weight: bold;">apol</span> wrote in <span style="color: #4b4d51; font-weight: bold;">CMakeLists.txt:29</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Would it make sense to have this option with the macro? This way we make sure it's easy to disable and we save some boilerplate on every project.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Yes, asked myself the same when doing all the patches for all the KF modules :)<br />
My answer to this has been:<br />
this might be too aggressive to non-KF projects which might want to use these macros, but with other option name, other default setting or even unconditionally.</p>

<p style="padding: 0; margin: 8px;">It rather points out another problem: there is right now massive cmake code duplication in all the KDE Frameworks modules.<br />
IMHO it would make more sense to invest into some ECMKFSetup & Co. macro set for de-duplicating all the repeated explicit logic across all the modules, also to keep & improve consistency between all the modules.</p>

<p style="padding: 0; margin: 8px;">I know this from other projects where it's successfully done, e.g. in Okteta for each of the currently 6 libs then to setup all the header installation, cmake/pkconfig/pri file generation code is as simple as:</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);">okteta_add_library(Gui NAMESPACE Okteta
    PUBLIC
        OktetaCore
        Qt5::Widgets
    PRIVATE
        KF5::I18n
        KF5::ConfigWidgets
    VERSION     ${OKTETAGUI_VERSION}
    SOVERSION   ${OKTETAGUI_SO_VERSION}
    ABIVERSION  ${OKTETALIBS_ABI_VERSION}
    SOURCES   ${oktetagui_LIB_SRCS}
    HEADERS   ${oktetagui_LIB_HDRS}
    CCHEADERS ${oktetagui_LIB_CCHDRS}
    NO_TARGET_NAMESPACE
    NO_VERSIONED_INCLUDEDIR
    NO_VERSIONED_PACKAGE_NAME
)

okteta_add_cmakeconfig(Gui NAMESPACE Okteta)

okteta_add_qmakeconfig(Gui NAMESPACE Okteta
    DEPS OktetaCore widgets
)

okteta_add_pkgconfig(Gui NAMESPACE Okteta
    DEPS OktetaCore Qt5Widgets
    DESCRIPTION "Okteta Gui"
)</pre></div>

<p style="padding: 0; margin: 8px;">And any improvements not needing new input data/arguments can be done centrally in the macro implementation, those which are just need visiting of all places to add any new arguments.</p>

<p style="padding: 0; margin: 8px;">Never proposed this so far, fearing the rather big work to implement that will get on my desk :P But very much think such KF-specific generic convenience macros would improve things for KDE Frameworks.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R302 KIconThemes</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D22790">https://phabricator.kde.org/D22790</a></div></div><br /><div><strong>To: </strong>kossebau, Frameworks<br /><strong>Cc: </strong>apol, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns<br /></div>