D11934: KDevelop : "Reparse Entire Project" action in the project manager context menu

[René J.V. Bertin]

rjvbb marked 3 inline comments as done.
rjvbb added inline comments.

INLINE COMMENTS

> mwolff wrote in parseprojectjob.h:36
> please don't do that. rather, reuse `ProjectController::reparseProject` below instead of creating the job manually

The idea was to move the "should we parse everything and why" logic outside of the function; it's not entirely relevant where the condition is moved to. Could be to this function's declaration, but could probably also be to the reparseProject definition.
The point is that parameter could then simply be called "parseAll" and would be easier to explain. You'd also get more control over what the function does; in particular trigger a partial reparse no matter what `parseAllProjectSources()` returns.

> mwolff wrote in projectcontroller.cpp:247
> deref the iterator returned by `QSet::begin()`
> 
> your QList won't work when you have multiple folders in a single project selected, you'll get a list with two entries, both pointing to the same project.

I'm dereferencing the QSet::constBegin() iterator, is that wrong (and if so, why is it allowed?)

> mwolff wrote in projectcontroller.cpp:1141
> it's not a bug fix, and you add new strings too

Well, outside of a freeze-before-a-release period of course...

> mwolff wrote in projectcontroller.cpp:1264
> expand this here

did you mean anything more than just passing the forceAll argument?

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190211/a71a4ac0/attachment.html>