Review Request: Check for possible unloading of version control plugin in ProjectPrivate.

Andreas Pakulat apaku at gmx.de
Fri Dec 24 22:44:45 UTC 2010


On 24.12.10 15:11:16, Milian Wolff wrote:
> On Tuesday 21 December 2010 14:06:39 Aleix Pol Gonzalez wrote:
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > http://git.reviewboard.kde.org/r/100225/#review592
> > -----------------------------------------------------------
> > 
> > Ship it!
> > 
> > 
> > Looks good to me.
> > Can we remove the hasError method now?
> 
> I've investigated it today and no, it's not possible. 
> 
> Or well, it would be possible if we only return QWeakPointers everywhere from 
> the PluginController. 
> 
> pluginManager->allPluginsForExtension( "org.kdevelop.IBasicVersionControl" )
> 
> That call in project.cpp:386 can lead to crashes if we unload the git plugin 
> via some Qt::QueuedConnection, even if I temporarily use a list of 
> QWeakPointers in allPluginsForExtension and convert it afterwards.
> 
> Anyhow, I don't find that API too bad, it's quite easy to use and works well. 
> So should we live with it?

IMHO asking a plugin wether it can be loaded/works correctly is
completely fine and totally correct. I would probably change the the
function name to something like 'checkDependencies' or 'canRun' or maybe
something else. IMHO hasErrors is not exactly a good name for a function
in which the plugin should check wether it can work, has all
dependencies needed etc. Oh and IIRC there was a second method to
transport a message-string or some such? That should simply be part of
the return value (i.e. make the result a small struct with type and
message).

Andreas

-- 
You should emulate your heros, but don't carry it too far.  Especially
if they are dead.




More information about the KDevelop-devel mailing list