----------------------------------------------------------- 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 > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
