kossebau added a comment.
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). 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... INLINE COMMENTS > kicontheme.h:292 > + KICONTHEMES_DEPRECATED_VERSION(5, 64, "No longer necessary") > + static void assignIconsToContextMenu(ContextMenus type, QList<QAction *> > actions); // TODO KF6 remove > #endif Is "// TODO KF6 remove" really needed BTW? Personally I would be strict and for KF6 just dump any API deprecated in KF5 times. Do you think there will be deprecated API we should keep around even in KF6? Or should this be considered on case-by-case once KF6 is created? REPOSITORY R302 KIconThemes REVISION DETAIL https://phabricator.kde.org/D24892 To: vkrause, kossebau Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
