Review Request 118643: Create an export filter for Wiki pages

Inge Wallin inge at lysator.liu.se
Tue Jun 10 17:41:15 BST 2014 

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


Found some naming issues, some spelling mistakes, missing comments and one missing feature (we don't handle default list styles).

Other than that I think that moji should add his copyright in a number of files since he wrote much of the actual wiki export.


filters/libodf2/KoOdfListStyle.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41595>

    Check in the spec if this is correct.  It feels like a copy&paste error.



filters/libodf2/KoOdfListStyle.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41596>

    also copy&paste error?



filters/libodf2/KoOdfStyleManager.h
<https://git.reviewboard.kde.org/r/118643/#comment41598>

    parameters should be const &



filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41601>

    Don't we need default list styles?



filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41603>

    Why this change?  It's probably correct but I'd like a comment about the reason for it.



filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41604>

    Should be disabled before merge (or disabled in master already).



filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41605>

    comment out kdebug's before merge



filters/libodf2/KoOdfStyleManager.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41606>

    Indeed.  I suggest we do this before the merge.



filters/libodfreader/OdfTextReader.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41609>

    Should be documented above



filters/libodfreader/OdfTextReader.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41610>

    Yep. Let's do it.



filters/libodfreader/OdfTextReader.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41611>

    This comment doesn't follow the template



filters/libodfreader/OdfTextReader.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41612>

    This comment doesn't follow the template



filters/libodfreader/OdfTextReader.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41613>

    This comment doesn't follow the template



filters/words/wiki/export/OdfReaderWikiContext.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41614>

    The name of this function seems wrong.  It is called ...StyleProperties but it returns a style.



filters/words/wiki/export/OdtReaderWikiBackend.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41615>

    -> checkHeadingLevel (camelCasing)
    
    Also, I don't like the name check<something> since there is no indication of what it checks, what the result is or what action should be taken. It's much too generic.



filters/words/wiki/export/OdtReaderWikiBackend.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41616>

    Strange indentation



filters/words/wiki/export/OdtReaderWikiBackend.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41617>

    Remove. This is from the text export filter.



filters/words/wiki/export/OdtReaderWikiBackend.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41618>

    spelling of the name...



filters/words/wiki/export/OdtReaderWikiBackend.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41619>

    camelCasing



filters/words/wiki/export/OdtReaderWikiBackend.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41620>

    I don't understand this. I think a better comment is warranted.



filters/words/wiki/export/OdtReaderWikiBackend.cpp
<https://git.reviewboard.kde.org/r/118643/#comment41621>

    spacing



words/part/CMakeLists.txt
<https://git.reviewboard.kde.org/r/118643/#comment41622>

    This change seems unnecessary :)


- Inge Wallin


On June 10, 2014, 3:52 p.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118643/
> -----------------------------------------------------------
> 
> (Updated June 10, 2014, 3:52 p.m.)
> 
> 
> Review request for Calligra, Camilla Boemann, Lassi Nieminen, mojtaba shahi, and Thorsten Zachmann.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> This patch contains an export filter for wiki pages from odt files. It's done by request of the KDE documentation people, mainly mamarok.
> 
> Note that not all of the wiki spec is supported yet. Most notably, tables are not supported yet but that is the main priority for the next feature inclusion. Also links needs more work. But there is support for lists, headers, some general formatting and of course text in general.
> 
> (Originally moji was going to put up this review but he got a pain in his hands so I offered to do it instead. I may have missed some important comment but I'm sure he will add to the comments then.)
> 
> 
> Diffs
> -----
> 
>   filters/libodf2/CMakeLists.txt 8829a3c 
>   filters/libodf2/KoOdfListLevelProperties.h PRE-CREATION 
>   filters/libodf2/KoOdfListLevelProperties.cpp PRE-CREATION 
>   filters/libodf2/KoOdfListStyle.h PRE-CREATION 
>   filters/libodf2/KoOdfListStyle.cpp PRE-CREATION 
>   filters/libodf2/KoOdfStyleManager.h 9bc51f4 
>   filters/libodf2/KoOdfStyleManager.cpp 7e558c7 
>   filters/libodf2/KoOdfStyleProperties.cpp f9d6d10 
>   filters/libodfreader/OdfTextReader.h f5636e4 
>   filters/libodfreader/OdfTextReader.cpp 31dfba5 
>   filters/libodfreader/OdfTextReaderBackend.h 4c82a02 
>   filters/libodfreader/OdfTextReaderBackend.cpp b6beb7d 
>   filters/words/CMakeLists.txt 805ca28 
>   filters/words/wiki/CMakeLists.txt PRE-CREATION 
>   filters/words/wiki/export/CMakeLists.txt PRE-CREATION 
>   filters/words/wiki/export/OdfReaderWikiContext.h PRE-CREATION 
>   filters/words/wiki/export/OdfReaderWikiContext.cpp PRE-CREATION 
>   filters/words/wiki/export/OdtReaderWikiBackend.h PRE-CREATION 
>   filters/words/wiki/export/OdtReaderWikiBackend.cpp PRE-CREATION 
>   filters/words/wiki/export/WikiExport.h PRE-CREATION 
>   filters/words/wiki/export/WikiExport.cpp PRE-CREATION 
>   filters/words/wiki/export/words_wiki_export.desktop PRE-CREATION 
>   filters/words/wiki/wiki-format.xml PRE-CREATION 
>   words/part/CMakeLists.txt 2bf1b0b 
> 
> Diff: https://git.reviewboard.kde.org/r/118643/diff/
> 
> 
> Testing
> -------
> 
> Lots of manual testing during development
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

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

More information about the calligra-devel mailing list