Review Request: Bibliography database UI

[Gopalakrishna Bhat]

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


I have not tried out this patch. Will try out this weekend. These comments are just by looking at the code.


libs/kotext/BibliographyGenerator.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11592>

    Use {} even for single line if-else code
    The structure is something like
    if (...) {
    //code here
    } else {
    //code here
    }



libs/kotext/KoInlineCite.h
<http://git.reviewboard.kde.org/r/105240/#comment11593>

    fieldName can be an enum value rater than a string



libs/kotext/KoInlineCite.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11599>

    Instead of all these if conditions we can use looping here. We can discuss about this on IRC



plugins/chartshape/TODO
<http://git.reviewboard.kde.org/r/105240/#comment11600>

    Are u sure these are ur changes?



plugins/textshape/CMakeLists.txt
<http://git.reviewboard.kde.org/r/105240/#comment11601>

    Space



plugins/textshape/dialogs/BibliographyDatabaseWindow.h
<http://git.reviewboard.kde.org/r/105240/#comment11602>

    space



plugins/textshape/dialogs/BibliographyDatabaseWindow.h
<http://git.reviewboard.kde.org/r/105240/#comment11603>

    Remove this second public



plugins/textshape/dialogs/BibliographyDatabaseWindow.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11604>

    We are well into 2012...the dooms year :)



plugins/textshape/dialogs/BibliographyDatabaseWindow.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11605>

    Use qDeleteAll



plugins/textshape/dialogs/BibliographyDb.h
<http://git.reviewboard.kde.org/r/105240/#comment11606>

    const QString & here and other places too



plugins/textshape/dialogs/BibliographyDb.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11607>

    Cant we use BibliographyDb::dbFields and build this string through looping?



plugins/textshape/dialogs/EditFiltersDialog.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11595>

    A comment describing what 62 is here



plugins/textshape/dialogs/EditFiltersDialog.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11596>

    (const QString &op) here and in other places too



plugins/textshape/dialogs/EditFiltersDialog.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11598>

    I do not see that u use m_filters anywhere other than the constructor. Hence this can be removed.



plugins/textshape/dialogs/EditFiltersDialog.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11608>

    Remove comment



plugins/textshape/dialogs/InsertCitationDialog.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11594>

    My first comment on the if block styling applies here and other places too


- Gopalakrishna Bhat


On June 13, 2012, 12:12 p.m., Smit Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105240/
> -----------------------------------------------------------
> 
> (Updated June 13, 2012, 12:12 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> Bibliography database UI.
> 1) create/load/save/edit table saved in SQLite file
> 2) Search in database
> 3) Apply custom filters to database
> 4) Integrated this UI with existing insert citation for edit/create cite in database
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 33cdabb 
>   libs/kotext/BibliographyGenerator.cpp 15808cb 
>   libs/kotext/KoInlineCite.h df4a2e9 
>   libs/kotext/KoInlineCite.cpp 6d949da 
>   libs/kotext/KoInlineTextObjectManager.cpp e82664d 
>   libs/kotext/KoTextEditor.cpp b3e16ba 
>   plugins/chartshape/TODO 94efb62 
>   plugins/textshape/CMakeLists.txt 6253323 
>   plugins/textshape/ReferencesTool.h 491b933 
>   plugins/textshape/ReferencesTool.cpp 564b658 
>   plugins/textshape/dialogs/BibliographyDatabaseWindow.h PRE-CREATION 
>   plugins/textshape/dialogs/BibliographyDatabaseWindow.cpp PRE-CREATION 
>   plugins/textshape/dialogs/BibliographyDatabaseWindow.ui PRE-CREATION 
>   plugins/textshape/dialogs/BibliographyDb.h PRE-CREATION 
>   plugins/textshape/dialogs/BibliographyDb.cpp PRE-CREATION 
>   plugins/textshape/dialogs/CitationInsertionDialog.h f5eac0b6 
>   plugins/textshape/dialogs/CitationInsertionDialog.cpp 14f9429 
>   plugins/textshape/dialogs/CitationInsertionDialog.ui 8f10f64 
>   plugins/textshape/dialogs/EditFiltersDialog.h PRE-CREATION 
>   plugins/textshape/dialogs/EditFiltersDialog.cpp PRE-CREATION 
>   plugins/textshape/dialogs/InsertCitationDialog.h PRE-CREATION 
>   plugins/textshape/dialogs/InsertCitationDialog.cpp PRE-CREATION 
>   plugins/textshape/dialogs/InsertCitationDialog.ui PRE-CREATION 
>   plugins/textshape/dialogs/SimpleCitationBibliographyWidget.cpp 2d1c1de 
>   plugins/textshape/dialogs/SimpleCitationBibliographyWidget.ui 55699bb 
> 
> Diff: http://git.reviewboard.kde.org/r/105240/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Smit Patel
> 
>

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