D22723: Fix RunnerManager::queryFinished()

[Fabian Vogt]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190726/fb1d8876/attachment-0001.html>