Review Request 113588: Create plugin system for semantic items, turn before hardcoded contact, event & location items into plugins

C. Boemann cbr at boemann.dk
Sun Nov 3 19:42:27 GMT 2013 

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


I like it and would like to see it shipped.

Just found a few cosmetic issues while reading through anyway


libs/rdf/KoRdfSemanticItemRegistry.h
<http://git.reviewboard.kde.org/r/113588/#comment31007>

    identation issues



plugins/semanticitems/location/KoRdfLocation.cpp
<http://git.reviewboard.kde.org/r/113588/#comment31008>

    spaces



plugins/semanticitems/location/KoRdfLocation.cpp
<http://git.reviewboard.kde.org/r/113588/#comment31009>

    spaces



plugins/semanticitems/location/KoRdfLocationEditWidget.h
<http://git.reviewboard.kde.org/r/113588/#comment31010>

    spaces



plugins/semanticitems/location/KoRdfLocationEditWidget.h
<http://git.reviewboard.kde.org/r/113588/#comment31011>

    spaces



plugins/semanticitems/location/KoRdfLocationTreeWidgetItem.cpp
<http://git.reviewboard.kde.org/r/113588/#comment31012>

    spaces



plugins/semanticitems/location/KoRdfLocationTreeWidgetItem.cpp
<http://git.reviewboard.kde.org/r/113588/#comment31013>

    spaces


- C. Boemann


On Nov. 3, 2013, 6:29 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113588/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2013, 6:29 p.m.)
> 
> 
> Review request for Calligra, C. Boemann and Boudewijn Rempt.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> Things work similar to before, so time to get you reviewing it, even if there are some things left for polishing, but that code was not polished before anyway.
> And at least the bugs I found also exist in master, hopefully will fix them after this went in, then also swing the polish tool a little more.
> 
> Can be tested as branch "rdf-turnIntoPlugins-kossebau".
> 
> Patches turn all places with hardcoded code for contact, event and location into generic algorithms, puts code for contact, event and location into separate plugins. Most code is just moved around, I did not try to change any logic other than needed for turning things into plugins. See the commits in the branch for the steps I did, more or less.
> 
> For now just a single new product "PLUGIN_SEMANTICITEMS", not sure if anyone would like that to be split up even more. Not yet sure which productsets we want that be part off. Also some README.packagers need to be added, not yet done.
> 
> 
> And lost two hours of beauty sleep on the following, see me in rage about it!!11!... ;)
> KoDocumentInfoDlg::addPageItem() is broken, the passed KoPageWidgetItem *item is no-where cached and never destructed. Results in crashes for KoDocumentRdfEditWidget (added by KWDocument::createDocumentInfoDialog(...), because it subscribes to docRdf, SIGNAL(semanticObjectUpdated(hKoRdfSemanticItem)) and if that is fired after the widget has already been closed, KoDocumentRdfEditWidget tries to access its UI which got deleted by KoDocumentInfoDlg already.
> There are also different casts between KoPageWidgetItem* and KoPageWidgetItem* which do not make sense, seems something was lost during rebase?
> Git blames commit d51556f8, "Split out the rdf code into its own library"...
> 
> 
> Diffs
> -----
> 
>   libs/rdf/KoRdfCalendarEventTreeWidgetItem.cpp 56a9f14 
>   libs/rdf/KoRdfFoaF.h 84de8cf 
>   libs/rdf/KoRdfCalendarEventTreeWidgetItem.h 8209962 
>   libs/rdf/KoRdfCalendarEventEditWidget.ui 2ad3024 
>   libs/rdf/KoRdfCalendarEvent.h 87e21b7 
>   libs/rdf/KoRdfCalendarEvent.cpp 710e9be 
>   libs/rdf/KoDocumentRdfEditWidget.ui ea343f8 
>   libs/rdf/KoDocumentRdfEditWidget.h 9420cea 
>   libs/rdf/KoDocumentRdfEditWidget.cpp 418e887 
>   libs/rdf/KoDocumentRdf.cpp 74ef469 
>   libs/rdf/InsertSemanticObjectCreateAction.cpp 1663c98 
>   libs/rdf/KoDocumentRdf.h 56cd996 
>   libs/rdf/CMakeLists.txt 24baf2e 
>   libs/kotext/KoTextInlineRdf.h 6d94c3f 
>   libs/kotext/KoTextRdfCore.cpp 4487b95 
>   CMakeLists.txt da4f9e3 
>   libs/rdf/KoRdfFoaF.cpp 20cac0c 
>   libs/rdf/KoRdfFoaFEditWidget.ui e938036 
>   libs/rdf/KoRdfFoaFTreeWidgetItem.h 47ae0ec 
>   libs/rdf/KoRdfFoaFTreeWidgetItem.cpp 9be7a86 
>   libs/rdf/KoRdfLocation.h f5a60f8 
>   libs/rdf/KoRdfLocation.cpp da3fdcd 
>   libs/rdf/KoRdfLocationEditWidget.h 8cc0086 
>   libs/rdf/KoRdfLocationEditWidget.cpp 5ca0d54 
>   libs/rdf/KoRdfLocationEditWidget.ui 8315aa5 
>   libs/rdf/KoRdfLocationEditWidgetMarble.ui 49dc790 
>   libs/rdf/KoRdfLocationTreeWidgetItem.h c37619a 
>   libs/rdf/KoRdfLocationTreeWidgetItem.cpp 35bf3b6 
>   libs/rdf/KoRdfLocationViewWidget.ui 42ada4d 
>   libs/rdf/KoRdfLocationViewWidgetMarble.ui 9323e4a 
>   libs/rdf/KoRdfSemanticItem.h 7959497 
>   libs/rdf/KoRdfSemanticItem.cpp 5b10fde 
>   libs/rdf/KoRdfSemanticItemFactoryBase.h PRE-CREATION 
>   libs/rdf/KoRdfSemanticItemFactoryBase.cpp PRE-CREATION 
>   libs/rdf/KoRdfSemanticItemRegistry.h PRE-CREATION 
>   libs/rdf/KoRdfSemanticItemRegistry.cpp PRE-CREATION 
>   libs/rdf/KoRdfSemanticTree.h 864fba7 
>   libs/rdf/KoRdfSemanticTree.cpp c824e02 
>   libs/rdf/KoRdfSemanticTreeWidgetItem.h 39dfbb2 
>   libs/rdf/KoRdfSemanticTreeWidgetItem.cpp 23d35ca 
>   libs/rdf/KoSemanticStylesheetsEditor.cpp 0c345cb 
>   libs/rdf/RdfForward.h b847027 
>   libs/rdf/RdfSemanticTreeWidgetAction.h c5aec3c 
>   libs/rdf/RdfSemanticTreeWidgetSelectAction.h 57e7e89 
>   libs/rdf/RdfSemanticTreeWidgetSelectAction.cpp 8788946 
>   libs/rdf/calligra_semanticitem.desktop PRE-CREATION 
>   libs/rdf/tests/TestSemanticItem.h 1a9bd60 
>   plugins/CMakeLists.txt fe07026 
>   plugins/semanticitems/CMakeLists.txt PRE-CREATION 
>   plugins/semanticitems/contact/CMakeLists.txt PRE-CREATION 
>   plugins/semanticitems/contact/KoContactSemanticItemFactory.h PRE-CREATION 
>   plugins/semanticitems/contact/KoContactSemanticItemFactory.cpp PRE-CREATION 
>   plugins/semanticitems/contact/KoContactSemanticItemPlugin.h PRE-CREATION 
>   plugins/semanticitems/contact/KoContactSemanticItemPlugin.cpp PRE-CREATION 
>   plugins/semanticitems/contact/KoRdfFoaF.h PRE-CREATION 
>   plugins/semanticitems/contact/KoRdfFoaF.cpp PRE-CREATION 
>   plugins/semanticitems/contact/KoRdfFoaFEditWidget.ui PRE-CREATION 
>   plugins/semanticitems/contact/KoRdfFoaFTreeWidgetItem.h PRE-CREATION 
>   plugins/semanticitems/contact/KoRdfFoaFTreeWidgetItem.cpp PRE-CREATION 
>   plugins/semanticitems/contact/Messages.sh PRE-CREATION 
>   plugins/semanticitems/contact/calligra_semanticitem_contact.desktop PRE-CREATION 
>   plugins/semanticitems/event/CMakeLists.txt PRE-CREATION 
>   plugins/semanticitems/event/KoEventSemanticItemFactory.h PRE-CREATION 
>   plugins/semanticitems/event/KoEventSemanticItemFactory.cpp PRE-CREATION 
>   plugins/semanticitems/event/KoEventSemanticItemPlugin.h PRE-CREATION 
>   plugins/semanticitems/event/KoEventSemanticItemPlugin.cpp PRE-CREATION 
>   plugins/semanticitems/event/KoRdfCalendarEvent.h PRE-CREATION 
>   plugins/semanticitems/event/KoRdfCalendarEvent.cpp PRE-CREATION 
>   plugins/semanticitems/event/KoRdfCalendarEventEditWidget.ui PRE-CREATION 
>   plugins/semanticitems/event/KoRdfCalendarEventTreeWidgetItem.h PRE-CREATION 
>   plugins/semanticitems/event/KoRdfCalendarEventTreeWidgetItem.cpp PRE-CREATION 
>   plugins/semanticitems/event/Messages.sh PRE-CREATION 
>   plugins/semanticitems/event/calligra_semanticitem_event.desktop PRE-CREATION 
>   plugins/semanticitems/location/CMakeLists.txt PRE-CREATION 
>   plugins/semanticitems/location/KoLocationSemanticItemFactory.h PRE-CREATION 
>   plugins/semanticitems/location/KoLocationSemanticItemFactory.cpp PRE-CREATION 
>   plugins/semanticitems/location/KoLocationSemanticItemPlugin.h PRE-CREATION 
>   plugins/semanticitems/location/KoLocationSemanticItemPlugin.cpp PRE-CREATION 
>   plugins/semanticitems/location/KoRdfLocation.h PRE-CREATION 
>   plugins/semanticitems/location/KoRdfLocation.cpp PRE-CREATION 
>   plugins/semanticitems/location/KoRdfLocationEditWidget.h PRE-CREATION 
>   plugins/semanticitems/location/KoRdfLocationEditWidget.cpp PRE-CREATION 
>   plugins/semanticitems/location/KoRdfLocationEditWidget.ui PRE-CREATION 
>   plugins/semanticitems/location/KoRdfLocationEditWidgetMarble.ui PRE-CREATION 
>   plugins/semanticitems/location/KoRdfLocationTreeWidgetItem.h PRE-CREATION 
>   plugins/semanticitems/location/KoRdfLocationTreeWidgetItem.cpp PRE-CREATION 
>   plugins/semanticitems/location/KoRdfLocationViewWidget.ui PRE-CREATION 
>   plugins/semanticitems/location/KoRdfLocationViewWidgetMarble.ui PRE-CREATION 
>   words/part/KWView.cpp c87c4df 
>   words/part/dockers/KWRdfDockerTree.cpp e8a4510 
>   words/part/tests/TestRdf.cpp 99b8f73 
> 
> Diff: http://git.reviewboard.kde.org/r/113588/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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

More information about the calligra-devel mailing list