<table><tr><td style="">vkrause 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><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D24892#552838" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D24892#552838</a>, <a href="https://phabricator.kde.org/p/kossebau/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@kossebau</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><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></div>
</blockquote>
<p>Assuming I used this correctly (setting EXCLUDE_DEPRECATED_BEFORE_AND_AT to 054000) this doesn't build independent of assignIconsToContextMenu, due to the Q_ENUM code generated for KIconLoader still referencing the deprecated FileSystem enum element?</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><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></blockquote>
<p>It has just been added and I assume people still need to get used to it and we are still ironing out the last issues, so maybe give it a bit more time before pulling it out again right away :) It does have BC and maintenance challenges for sure, but IMHO that is ok as I don't really see it as something to use in production rather than as a tool to help with migrations to new major version.</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;">kossebau</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kicontheme.h:292</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><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 style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I am unsure about that, therefore I'm opting for the safe approach :) It is IMHO possible some deprecate API will stay, e.g. if it got deprecated before having strict rules on porting away from such API or the porting impact turning out more complex than anticipated. I'm therefore only adding the KF6 todos in places where they could be executed immediately when disregarding BC.</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>