Review Request 111990: make calligra plugins kxmlguiclient instead of kpart

Sun Aug 11 04:59:47 BST 2013

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111990/#review37493
-----------------------------------------------------------

First quick review:

braindump/plugins/quickstates/BraindumpQuickStatesPlugin.h
<http://git.reviewboard.kde.org/r/111990/#comment27716>

    What about adding a class CalligraPlugin : public QObject, public XMLGUIClient from which all the existing plugin classes derive instead of KParts::Plugin?
    Not that there is any additional method we could add to this intermediate class ATM.
    But it would remove the need to always deal with QObject and KXMLGUIClient base classes, like in the plugin declarations and the plugin creation code,
            KXMLGUIClient* plugin =
    service->createInstance<KParts::Plugin> (this, QVariantList(), &error);

braindump/plugins/quickstates/BraindumpQuickStatesPlugin.cpp
<http://git.reviewboard.kde.org/r/111990/#comment27715>

    No forward-passing of the parent object to the QObject?
    I wonder who is supposed to do memory management of that plugin, if not the parent, which is the View object, as passed in View::loadExtensions() to 
    service->createInstance<QObject>(this, QVariantList(), &error))?

    The insertChildClient(...) call does not result in memory parentship, if that was assumed here.

karbon/plugins/flattenpath/FlattenPathPlugin.cpp
<http://git.reviewboard.kde.org/r/111990/#comment27717>

    karbon/karbonplugins -> karbon/plugins or karbon/viewplugins?

    karbon/karbonplugins seems redundant

karbon/plugins/refinepath/RefinePathPlugin.cpp
<http://git.reviewboard.kde.org/r/111990/#comment27718>

    Memory management?

- Friedrich W. H. Kossebau

On Aug. 10, 2013, 1:36 p.m., Boudewijn Rempt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111990/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2013, 1:36 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> This is a small part of my MVC refactoring branch -- the plugin cleanup. For historical reasons, many of Calligra's plugins were created as kparts, while they are, in fact, only kxmlguiclients, not parts at at all. This patch cleans that up.
> 
> 
> Diffs
> -----
> 
>   braindump/plugins/quickstates/BraindumpQuickStatesPlugin.h b9625b0 
>   braindump/plugins/quickstates/BraindumpQuickStatesPlugin.cpp bc27946 
>   braindump/src/View.cpp ef19171 
>   devtools/cstester/cstester.cpp b88619b 
>   extras/okularodpgenerator/OkularOdpGenerator.cpp 94650bf 
>   karbon/data/CMakeLists.txt 6b1e329 
>   karbon/data/karbon_module.desktop 7acc680 
>   karbon/data/karbon_plugin.desktop PRE-CREATION 
>   karbon/plugins/flattenpath/CMakeLists.txt f83620b 
>   karbon/plugins/flattenpath/FlattenPathPlugin.h 622973c 
>   karbon/plugins/flattenpath/FlattenPathPlugin.cpp d03ccca 
>   karbon/plugins/flattenpath/karbonflattenpath.desktop PRE-CREATION 
>   karbon/plugins/refinepath/CMakeLists.txt caf6aa7 
>   karbon/plugins/refinepath/RefinePathPlugin.h be16e29 
>   karbon/plugins/refinepath/RefinePathPlugin.cpp b951adc 
>   karbon/plugins/refinepath/karbonrefinepath.desktop PRE-CREATION 
>   karbon/plugins/roundcorners/CMakeLists.txt 82b7159 
>   karbon/plugins/roundcorners/RoundCornersPlugin.h 5cd610a 
>   karbon/plugins/roundcorners/RoundCornersPlugin.cpp 7ac63bf 
>   karbon/plugins/roundcorners/karbonroundcorners.desktop PRE-CREATION 
>   karbon/plugins/whirlpinch/CMakeLists.txt 2e920e2 
>   karbon/plugins/whirlpinch/WhirlPinchPlugin.h 51280a1 
>   karbon/plugins/whirlpinch/WhirlPinchPlugin.cpp 94a3527 
>   karbon/plugins/whirlpinch/karbonwhirlpinch.desktop PRE-CREATION 
>   karbon/ui/KarbonFactory.cpp 8bbabbe 
>   karbon/ui/KarbonView.cpp 068d39d 
>   karbon/ui/dockers/KarbonLayerDocker.cpp 1cf8395 
>   krita/plugins/extensions/shiva/shivagenerator.h 55894f4 
>   krita/plugins/paintops/libbrush/kis_brush_registry.cpp f54a619 
>   krita/plugins/paintops/particle/particle_paintop_plugin.h bd6b245 
>   krita/ui/kis_factory2.h 888b742 
>   krita/ui/kis_factory2.cc b7896e5 
>   krita/ui/kis_paintop_box.cc 667e6b5 
>   krita/ui/kis_view2.cpp 6af9f4b 
>   krita/ui/kis_view_plugin.h ff19b2a 
>   krita/ui/kis_view_plugin.cpp 69f5b9d 
>   libs/kokross/KoScriptingPart.h 4f4c933 
>   libs/kokross/KoScriptingPart.cpp 09b8cbb 
>   libs/kopageapp/KoPAView.cpp bbb068a 
>   libs/main/CMakeLists.txt 9b8503b 
>   libs/main/KoDocument.cpp 1738778 
>   libs/main/KoFactory.h dc4ada0 
>   libs/main/KoFactory.cpp c361a11 
>   libs/main/KoMainWindow.cpp 12423a7 
>   libs/main/KoServiceProvider.h 35d603a 
>   libs/main/KoServiceProvider.cpp f063401 
>   libs/main/KoView.h 10b6403 
>   plan/CMakeLists.txt 4ff06eb 
>   plan/kptmaindocument.cpp 333310c 
>   plan/kptview.cpp 537e047 
>   plan/plan_viewplugin.desktop PRE-CREATION 
>   plan/plugins/scripting/CMakeLists.txt 836176d 
>   plan/plugins/scripting/ScriptingPart.cpp e6d6004 
>   plan/plugins/scripting/planscripting.desktop PRE-CREATION 
>   plugins/colorengines/lcms2/LcmsEnginePlugin.h 36a5497 
>   plugins/staging/pivottables/CMakeLists.txt efb2753 
>   plugins/staging/pivottables/pivotplugin.h 17fc384 
>   plugins/staging/pivottables/pivotplugin.cpp 1bef7d2 
>   plugins/staging/pivottables/sheetspivottables.desktop PRE-CREATION 
>   sheets/CMakeLists.txt e0f4a02 
>   sheets/part/View.cpp 0dc055a 
>   sheets/plugins/scripting/CMakeLists.txt 27e3861 
>   sheets/plugins/scripting/ScriptingPart.h ac9e614 
>   sheets/plugins/scripting/ScriptingPart.cpp 26431b0 
>   sheets/plugins/scripting/sheetsscripting.desktop PRE-CREATION 
>   sheets/plugins/solver/CMakeLists.txt ad813e6 
>   sheets/plugins/solver/Solver.h 21ee006 
>   sheets/plugins/solver/Solver.cpp c87639e 
>   sheets/plugins/solver/sheetssolver.desktop PRE-CREATION 
>   sheets/sheets_viewplugin.desktop PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/111990/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Boudewijn Rempt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130811/fbbb5240/attachment.htm>