Review Request 121881: Greatly cleanup PluginController::loadPluginInternal.

Sergey Kalinichev kalinichev.so.0 at gmail.com
Wed Jan 7 11:22:37 UTC 2015


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


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.

- Sergey Kalinichev


On Jan. 6, 2015, 10: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, 10: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/3848bb2c/attachment.html>


More information about the KDevelop-devel mailing list