Review Request 121775: Correctly load dependencies for plugins

Milian Wolff mail at milianw.de
Sat Jan 3 14:23:03 UTC 2015


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


I wonder why I never saw this issue. Can you explain what you are encountering? Also, adding a unit test would help here to ensure we never run into this issue again.

Generally, the code for checkForDependencies ensures that all dependencies are available and none are disabled. Only then should potential dependencies be loaded. Your code now always tries to load dependencies, no? Even when one of them is disabled or not existing.

so -1 from my side until you give more information on what issue you are encountering and trying to solve.


shell/plugincontroller.cpp
<https://git.reviewboard.kde.org/r/121775/#comment50812>

    pass the missing dependencies here, otherwise you don't need the dependency list here at all



shell/plugincontroller.cpp
<https://git.reviewboard.kde.org/r/121775/#comment50811>

    just return the QSet<QString>, no? why converting it to a list? Or just return a bool and rename this to "hasMissingDependencies" or such?


- Milian Wolff


On Dec. 31, 2014, 12:34 p.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121775/
> -----------------------------------------------------------
> 
> (Updated Dec. 31, 2014, 12:34 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Currently if some plugin depends on another one that's not yet loaded, the former one won't be loaded too.
> 
> This happens because checkForDependencies returns false if there are some dependencies, but the code that calls this method threats false return code as an error, so it doesn't try to load any dependencies at all.
> 
> 
> Diffs
> -----
> 
>   shell/plugincontroller.h 89b0b8b 
>   shell/plugincontroller.cpp f70fc64 
> 
> Diff: https://git.reviewboard.kde.org/r/121775/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

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


More information about the KDevelop-devel mailing list