fvogt added a comment.
In D22723#502365 <https://phabricator.kde.org/D22723#502365>, @aacid wrote: > I honestly don't see the problem with this patch, one may argue that the ThreadWeaver API is awkward, ok, but this is using it correctly AFAICS, i.e. have a ThreadWeaver::QObjectDecorator, give it a ThreadWeaver::Job on its constructor, and go on from there. Correctly as in "it's supposed to work" yes, but it's not as it was intended to be AFAICT. This now adds a custom private and friend class (ugh) which means that now there's a sandwich out of three classes: `FindMatchesJob -(inherits)> QObjectDecorator -(uses)> FindMatchesJobInternal` while only a single one would do the job. I did a PoC of dropping use of `QObjectDecorator`: https://phabricator.kde.org/D22758 Here, one may argue that it reinvents the wheel, but if the current iteration of the wheel is square I'd say it's allowed. It also gets rid of a heap allocation. What do you think? If you don't like it I won't object landing this internal class, but please add a long comment explaining why it was done like this. REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D22723 To: apol, #frameworks, fvogt, davidedmundson Cc: aacid, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns
