vkrause added a comment.
In D24892#554048 <https://phabricator.kde.org/D24892#554048>, @kossebau wrote: > In D24892#553597 <https://phabricator.kde.org/D24892#553597>, @vkrause wrote: > > > In D24892#552838 <https://phabricator.kde.org/D24892#552838>, @kossebau wrote: > > > > > 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). > > > > > > Assuming I used this correctly (setting EXCLUDE_DEPRECATED_BEFORE_AND_AT to 054000) > > > Hm, guess the "default:0" confuses people, EXCLUDE_DEPRECATED_BEFORE_AND_AT would be set as string in majpr.minor.patch format ("0" & "CURRENT" are special cases). That is documented on https://api.kde.org/ecm/module/ECMGenerateExportHeader.html but seemingly that is too far away, so will add to the option description text. "default:0" is not what confused me, but `EXCLUDE_DEPRECATED_BEFORE_AND_AT set to hexnumber(5.64.0)` in your original statement ;-) >> this doesn't build independent of assignIconsToContextMenu, due to the Q_ENUM code generated for KIconLoader still referencing the deprecated FileSystem enum element? > > Building with EXCLUDE_DEPRECATED_BEFORE_AND_AT set to 0, 4.8.0, 5.0.0 & CURRENT, to cover before & after versions, the build does not fail here. Doing a clean build and setting EXCLUDE_DEPREACTED_BEFORE_AND_AT at the *first* cmake run makes it work here reliably too. So, to get back to the original question: yes, this patch fixes the build :) INLINE COMMENTS > kossebau wrote in kicontheme.h:292 > Some API deprecated staying because porting away is not easy to do should be > the exception though, no? > I would rather flag only such API elements with a comment once it is found > out in real world that porting is more hard than anticipated. > But at the point of time of deprecation, the whole idea of the warnings being > inserted into the compiler is to have people stop using it, so it can be > removed at the next occasion, no? Otherwise the whole warning is harmful > noise making people ignore warnings. > > And ideally the concept of deprecated-but-might-stay should be codified > ideally in the macros, so the compiler can be taught to do what is wanted. Going forward certainly yes. What I have doubts about in this regard are the things that got deprecated in the past, before we had clear guidelines for this. REPOSITORY R302 KIconThemes BRANCH master REVISION DETAIL https://phabricator.kde.org/D24892 To: vkrause, kossebau Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
