----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104235/#review11304 -----------------------------------------------------------
Looks very good. I have added some style issues that should be fixed. After that I think it is good for inclusion. The filename of the classes should use camelcase. So the classes should be renamed to match the case of the class e.g. barcodeshape.cpp -> BarcodeShape.cpp . plugins/barcodeshape/barcodeshape.cpp <http://git.reviewboard.kde.org/r/104235/#comment9057> Please move the type to be in the same line as the function. plugins/barcodeshape/barcodeshape.cpp <http://git.reviewboard.kde.org/r/104235/#comment9056> The indention is wrong here. plugins/barcodeshape/barcodeshape.cpp <http://git.reviewboard.kde.org/r/104235/#comment9059> There should be a blank after , plugins/barcodeshape/barcodeshape.cpp <http://git.reviewboard.kde.org/r/104235/#comment9055> I think this should be written better inside a switch statement. Is the static_cast really needed? plugins/barcodeshape/barcodeshape.h <http://git.reviewboard.kde.org/r/104235/#comment9062> Is there a reason to make this a QObject? I did not see it it. If not please remove. plugins/barcodeshape/barcodeshape.h <http://git.reviewboard.kde.org/r/104235/#comment9054> One public, protected is enough. Please remove the additional ones. plugins/barcodeshape/barcodeshape.h <http://git.reviewboard.kde.org/r/104235/#comment9058> In calligra we use m_ prefix for member variables. plugins/barcodeshape/barcodeshapeconfigcommand.h <http://git.reviewboard.kde.org/r/104235/#comment9060> Indention wrong, please remove one blank. plugins/barcodeshape/barcodeshapeconfigcommand.h <http://git.reviewboard.kde.org/r/104235/#comment9061> Members should use m_ prefix plugins/barcodeshape/barcodeshapefactory.cpp <http://git.reviewboard.kde.org/r/104235/#comment9063> This text might be to long for showing in the Add Shape docker. How about changing the order to Barcode 1/2 D then at least barcode should be fully visible in the docker. - Thorsten Zachmann On March 12, 2012, 1:57 a.m., Friedrich W. H. Kossebau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104235/ > ----------------------------------------------------------- > > (Updated March 12, 2012, 1:57 a.m.) > > > Review request for Calligra. > > > Description > ------- > > Adds a shape for barcodes, based on the lib prison. > Adding, editing, saving and loading seems to work. > For non-Calligra consumers of the OpenDocument format a fallback image is > stored. > > > Diffs > ----- > > plugins/CMakeLists.txt 69e5d96 > plugins/barcodeshape/CMakeLists.txt PRE-CREATION > plugins/barcodeshape/Messages.sh PRE-CREATION > plugins/barcodeshape/barcodeshape.cpp PRE-CREATION > plugins/barcodeshape/barcodeshape.desktop PRE-CREATION > plugins/barcodeshape/barcodeshape.h PRE-CREATION > plugins/barcodeshape/barcodeshapeconfigcommand.h PRE-CREATION > plugins/barcodeshape/barcodeshapeconfigcommand.cpp PRE-CREATION > plugins/barcodeshape/barcodeshapeconfigwidget.h PRE-CREATION > plugins/barcodeshape/barcodeshapeconfigwidget.cpp PRE-CREATION > plugins/barcodeshape/barcodeshapefactory.h PRE-CREATION > plugins/barcodeshape/barcodeshapefactory.cpp PRE-CREATION > plugins/barcodeshape/barcodeshapeplugin.h PRE-CREATION > plugins/barcodeshape/barcodeshapeplugin.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/104235/diff/ > > > Testing > ------- > > Created, edited and deleted barcode shapes. Saved files with barcode shapes > and loaded them again. > > > Thanks, > > Friedrich W. H. Kossebau > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
