<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/D24892">View Revision</a></tr></table><br /><div><div><p>Looks good. Perfect would be if you tested KIconThemes with EXCLUDE_DEPRECATED_BEFORE_AND_AT set to hexnumber(5.64.0), to see if there is no internal usage still happening, like with autotests which might need to support such a build with KICONTHEMES_ENABLE_DEPRECATED_SINCE (ENABLE variant, as external to lib).</p>
<p>In general, given even you missed this (both the build disabling of the implementation as well as testing the build with EXCLUDE_DEPRECATED_BEFORE_AND_AT, I asume), I wonder if the support for build-without-deprecated should not be removed again from KDE Frameworks (though not the ECM macro, it works and others might find it useful). KF contributors/builders might not really need it or gain from it before actually KF6 gets created, and things are fragile enough with the new deprecation macros even without that feature (think virtual methods & enums being accidentally wrapped by *_ENABLE_DEPRECATED_SINCE...</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/D24892#inline-140529">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kicontheme.h:292</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(251, 175, 175, .7);"> <span class="n">KICONTHEMES_DEPRECATED_VERSION</span><span class="p">(</span><span style="color: #601200">5</span><span class="p">,</span> <span style="color: #601200">64</span><span class="p">,</span> <span style="color: #766510">"No longer necessary"</span><span class="p">)<span class="bright"></span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">static</span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">void</span></span><span class="bright"> </span><span class="n"><span class="bright">assignIconsToContextMenu</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">ContextMenus</span></span><span class="bright"> </span><span class="n"><span class="bright">type</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">QList</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright"><</span></span><span class="bright"></span><span class="n"><span class="bright">QAction</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">*></span></span><span class="bright"> </span><span class="n"><span class="bright">actions</span></span><span class="bright"></span><span class="p"><span class="bright">);</span></span><span class="bright"> </span><span style="color: #74777d"><span class="bright">// TODO KF6 remove</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">KICONTHEMES_DEPRECATED_VERSION</span><span class="p">(</span><span style="color: #601200">5</span><span class="p">,</span> <span style="color: #601200">64</span><span class="p">,</span> <span style="color: #766510">"No longer necessary"</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">static</span> <span style="color: #aa4000">void</span> <span class="n">assignIconsToContextMenu</span><span class="p">(</span><span class="n">ContextMenus</span> <span class="n">type</span><span class="p">,</span> <span class="n">QList</span><span style="color: #aa2211"><</span><span class="n">QAction</span> <span style="color: #aa2211">*></span> <span class="n">actions</span><span class="p">);</span> <span style="color: #74777d">// TODO KF6 remove</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #304a96">#endif</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Is "// TODO KF6 remove" really needed BTW?</p>
<p style="padding: 0; margin: 8px;">Personally I would be strict and for KF6 just dump any API deprecated in KF5 times.<br />
Do you think there will be deprecated API we should keep around even in KF6?<br />
Or should this be considered on case-by-case once KF6 is created?</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/D24892">https://phabricator.kde.org/D24892</a></div></div><br /><div><strong>To: </strong>vkrause, kossebau<br /><strong>Cc: </strong>kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns<br /></div>