<table><tr><td style="">dfaure 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/D28765">View Revision</a></tr></table><br /><div><div><p>Thanks for the review! I was getting desperate to get one ;-)</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/D28765#inline-166589">View Inline</a><span style="color: #4b4d51; font-weight: bold;">svuorela</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kcmoduleinfo.cpp:73</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">At a later point, I*m not sure what the purpose is for these members are - but that's probably for another changeset.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Right, I was wondering the same. We could just implement the getters so that they call these things directly.</p>

<p style="padding: 0; margin: 8px;">But then I'm also wondering what's the purpose of KCModuleInfo at all and whether we could just use KPluginInfo directly instead -- but that's a KF6 consideration, since it's part of the API for KCMultiDialog and others.</p>

<p style="padding: 0; margin: 8px;">Well, we could add a addModule(KPluginInfo) overload if that's the direction we're going for; I just don't have full overview on the KCM stuff.</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/D28765#inline-166590">View Inline</a><span style="color: #4b4d51; font-weight: bold;">svuorela</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kcmoduleinfo.h:131</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Can we mark it as deprecated?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It's complicated.</p>

<ol class="remarkup-list">
<li class="remarkup-list-item">If you use the QString constructor, you know service() is usable. That's the case for all users of KCModuleInfo except KCModuleLoader. [Not that there are many]</li>
</ol>

<ol class="remarkup-list" start="2">
<li class="remarkup-list-item">Even KCModuleLoader calls service(), to test for noDisplay().</li>
</ol>

<p style="padding: 0; margin: 8px;">The whole concept of NoDisplay only makes sense for desktop files, unless we want to have that in JSON metadata as well. I suppose we do, but this currently seems to be completely missing (e.g. from KPluginMetaData, if we want this in all plugins, not just KCMs). We'd have to duplicate the logic currently in KService::noDisplay().</p>

<p style="padding: 0; margin: 8px;">Any volunteers to look into this? :-)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R295 KCMUtils</div></div></div><br /><div><strong>BRANCH</strong><div><div>master</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28765">https://phabricator.kde.org/D28765</a></div></div><br /><div><strong>To: </strong>dfaure, pino, broulik, mart, davidedmundson, ngraham, svuorela<br /><strong>Cc: </strong>svuorela, cblack, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns<br /></div>