D8059: KDevelop: abstractfilemanagerimport benchmark

Milian Wolff noreply at phabricator.kde.org
Thu Nov 2 21:48:05 UTC 2017


mwolff added a comment.


  In https://phabricator.kde.org/D8059#160968, @rjvbb wrote:
  
  > So, where do we go from here?
  >
  > I started preparing the introduction of the ProjectWatcher class (which was rejected from the benchmark introduction patch) but then realised that it isn't justified as a standalone commit. That class only makes sense if you're going to use it other than by handing the project top dir to ProjectWatcher::addDir, telling it to add everything under that directory.
  >
  > The class makes sense as part of my patch that lets FileManagerListJob determine which directories (dirs only!) get watched, and it's with that use pattern that it makes sense to check if the directory trips a filter.
  >
  > IOW, I'm not in favour of going through a review just to introduce a ProjectWatcher class knowing it does things (like locking a mutex) that are superfluous in the current context.
  >
  > As to checking the project filter in ProjectWatcher::addDir() : it seems obvious that this *must* have a significant benefit under the right conditions (in-tree build dir and/or the .git directory for projects with a matching exclusion filter). Is it really necessary to go through an additional review just to add the filter check?
  
  
  Fair enough, I see the point you are making. Feel free to update the existing patch that changes the watcher, then we can continue from there

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D8059

To: rjvbb, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel, njensen, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171102/03fd969d/attachment-0001.html>


More information about the KDevelop-devel mailing list