Review Request 121881: Greatly cleanup PluginController::loadPluginInternal.

[Milian Wolff]

> On Jan. 7, 2015, 11:22 a.m., Sergey Kalinichev wrote:
> > Still see no reason why you didn't remove checkForDependencies? IMO it makes things more complicated. You can do the same by using just  loadDependencies.
> > Or at least you could have added a doxygen comment for checkForDependencies, or renamed it to e.g. hasUnresolvedDependencies. Because currently it's not clear what it does, and what returns in each situation.

Renaming it is a good idea indeed, and I'll also extend the comments.

The reason it exists is that loadDependencies does just that, i.e. it loads all dependencies. But assume the following is the case:

Plugin A depends on B, C, D.

Assume D does not exist (not installed) or is explicitly disabled by the user

Now, if we'd just use loadDependencies, then B and C would be loaded and then D fails, so A also fails. This is not good, we don't want to load B nor C b/c we know D is going to fail and thus also A. This is what the checkForDependencies step does. Does this make things clearer?


- Milian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121881/#review73349
-----------------------------------------------------------


On Jan. 6, 2015, 6:36 p.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121881/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2015, 6:36 p.m.)
> 
> 
> Review request for KDevelop and Sergey Kalinichev.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> This hopefully resolves some issues in understanding the flow of
> this method. Some code could also be removed and the warning
> messages better formatted.
> 
> 
> Diffs
> -----
> 
>   shell/plugincontroller.cpp f70fc646e00e257bc0cd0a401cd8106569e23181 
> 
> Diff: https://git.reviewboard.kde.org/r/121881/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Milian Wolff
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150107/dc81b5d2/attachment.html>