D29281: Deprecate defunct functions

alex added a comment. Ufff, never thought sth. simple like deprecating a few functions could cause such an issue! @davidre Thanks a lot for fixing this! REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D29281 To: alex, #plasma, broulik, davidedmundson, vkrause,

D29281: Deprecate defunct functions

kossebau added a comment. In D29281#675955 , @davidre wrote: > This caused https://bugs.kde.org/show_bug.cgi?id=423003. I removed excluding the virtual method from the build in >

D29281: Deprecate defunct functions

davidre added a comment. This caused https://bugs.kde.org/show_bug.cgi?id=423003. I removed excluding the virtual method from the build in https://invent.kde.org/frameworks/krunner/commit/8f7ce559b84ee0c21de0256e6591793e4b95f411 REPOSITORY R308 KRunner REVISION DETAIL

D29281: Deprecate defunct functions

alex closed this revision. REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D29281 To: alex, #plasma, broulik, davidedmundson, vkrause, meven Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

meven accepted this revision as: meven. meven added a comment. This revision is now accepted and ready to land. Nice REPOSITORY R308 KRunner BRANCH deprecations (branched from master) REVISION DETAIL https://phabricator.kde.org/D29281 To: alex, #plasma, broulik, davidedmundson,

D29281: Deprecate defunct functions

alex marked an inline comment as done. REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D29281 To: alex, #plasma, broulik, davidedmundson, vkrause, meven Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

alex updated this revision to Diff 83119. alex added a comment. Fix typo in version number REPOSITORY R308 KRunner CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29281?vs=83115=83119 BRANCH deprecations (branched from master) REVISION DETAIL

D29281: Deprecate defunct functions

alex added inline comments. INLINE COMMENTS > meven wrote in querymatch.cpp:338 > Did you really need to remove this code ? The question this method should answer is: Does this match have a configuration interface? And the answer is no, because the functionality does not exist anymore. And if

D29281: Deprecate defunct functions

meven added inline comments. INLINE COMMENTS > querymatch.cpp:338 > { > -return d->runner && d->runner.data()->hasRunOptions(); > } Did you really need to remove this code ? > querymatch.cpp:343 > > +#if KRUNNER_BUILD_DEPRECATED_SINCE(5, 1) > void

D29281: Deprecate defunct functions

alex added inline comments. INLINE COMMENTS > kossebau wrote in abstractrunner.h:154 > Should be "5.0", not "5,0" here in the API dox text and elsewhere, no? (dot > instead of comma) Yes. I accidentally did it this way, because in German you use a comma :D > kossebau wrote in

D29281: Deprecate defunct functions

alex marked 9 inline comments as done. REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D29281 To: alex, #plasma, broulik, davidedmundson, vkrause, meven Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

alex updated this revision to Diff 83115. alex added a comment. Fix comma, KRUNNER_DEPRECATED_VERSION_BELATED macro REPOSITORY R308 KRunner CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29281?vs=83110=83115 BRANCH deprecations (branched from master) REVISION DETAIL

D29281: Deprecate defunct functions

kossebau added inline comments. INLINE COMMENTS > abstractrunner.h:154 > * is called, the runner should return true > + * @deprecated Since 5,0, this feature has been defunct > */ Should be "5.0", not "5,0" here in the API dox text and elsewhere, no? (dot instead of

D29281: Deprecate defunct functions

alex added a reviewer: meven. REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D29281 To: alex, #plasma, broulik, davidedmundson, vkrause, meven Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

alex updated this revision to Diff 83110. alex added a comment. Change deprecation version from 5.70 to 5.71 REPOSITORY R308 KRunner CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29281?vs=81546=83110 BRANCH deprecations (branched from master) REVISION DETAIL

D29281: Deprecate defunct functions

kossebau added a comment. Looks good to me now, just caring about the use of the deprecation macros (no clue about the actual code, has to be Kai or someone else with krunner knowledge (if there is) to review). Discussing this I see a small flaw in the case of retrospective deprecation

D29281: Deprecate defunct functions

alex updated this revision to Diff 81546. alex added a comment. Wrap macro about single method, change deprecation text And thanks for the feedback and explanations :-). REPOSITORY R308 KRunner CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29281?vs=81543=81546 BRANCH

D29281: Deprecate defunct functions

kossebau added inline comments. INLINE COMMENTS > kossebau wrote in abstractrunner.cpp:221 > Given EXCLUDE_DEPRECATED_BEFORE_AND_AT is used with KRunner, you want to also > wrap all the implementations of the now deprecated API with the BUILD variant > of the macro, so here for example: > >

D29281: Deprecate defunct functions

alex updated this revision to Diff 81543. alex added a comment. Wrap implementations REPOSITORY R308 KRunner CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29281?vs=81539=81543 BRANCH deprecations (branched from master) REVISION DETAIL https://phabricator.kde.org/D29281

D29281: Deprecate defunct functions

alex added inline comments. INLINE COMMENTS > kossebau wrote in abstractrunner.h:154 > The text in the API dox can stay "Since 5.0". > > The only crititical thing where the number of an older, already released > version should not be used is the KRUNNER_ENABLE_DEPRECATED_SINCE macro. >

D29281: Deprecate defunct functions

kossebau added a comment. Looks good, almost :) It's a bit complicated on first use (well, also still ater, but no better solution was found to get version-controlled API deprecation with C++ for now). INLINE COMMENTS > abstractrunner.cpp:221 > > void

D29281: Deprecate defunct functions

alex updated this revision to Diff 81539. alex added a comment. Use mordern macros Thanks :-) REPOSITORY R308 KRunner CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29281?vs=81528=81539 BRANCH deprecations (branched from master) REVISION DETAIL

D29281: Deprecate defunct functions

kossebau added a comment. Be aware that just adding a plain KRUNNER_DEPRECATED is not up-to-modern KF standards :) Please see cf5f7b4040a77ae69452d58bc189dcc3baaedd92 what macros there are now and how to apply

D29281: Deprecate defunct functions

alex updated this revision to Diff 81528. alex added a comment. Add missing KRUNNER_DEPRECATED REPOSITORY R308 KRunner CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29281?vs=81527=81528 BRANCH deprecations (branched from master) REVISION DETAIL

D29281: Deprecate defunct functions

alex created this revision. alex added reviewers: Plasma, broulik, davidedmundson, vkrause. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. alex requested review of this revision. REVISION SUMMARY As in T12163 explained