Review Request: New shape for 1-D and 2-D barcodes

[Friedrich W. H. Kossebau]

> On March 12, 2012, 4:27 a.m., Thorsten Zachmann wrote:
> > plugins/barcodeshape/barcodeshape.cpp, lines 206-210
> > <http://git.reviewboard.kde.org/r/104235/diff/1/?file=52496#file52496line206>
> >
> >     I think this should be written better inside a switch statement. Is the static_cast really needed?

static_cast is only needed for that kind of assignement pattern, as the type of the values needs to be the same, no casting done by the compiler. But changed to switch now.


> On March 12, 2012, 4:27 a.m., Thorsten Zachmann wrote:
> > plugins/barcodeshape/barcodeshape.h, line 60
> > <http://git.reviewboard.kde.org/r/104235/diff/1/?file=52498#file52498line60>
> >
> >     One public, protected is enough. Please remove the additional ones.

Okay, changed to match the rest of the Calligra code. Though IMHO this (commented) grouping of the API really helps to understand what a class does. Well, will not argue about it for now, as long as I stay just an outside contributor to Calligra ;)


> On March 12, 2012, 4:27 a.m., Thorsten Zachmann wrote:
> > plugins/barcodeshape/barcodeshapeconfigcommand.h, lines 36-38
> > <http://git.reviewboard.kde.org/r/104235/diff/1/?file=52499#file52499line36>
> >
> >     Indention wrong, please remove one blank.

Hm, could not see where the indention is wrong (perhaps fixed it already here). Should the lines be indented by 4-space-tabs, or by the first parameter in the first line (what I tried to do)?


> On March 12, 2012, 4:27 a.m., Thorsten Zachmann wrote:
> > plugins/barcodeshape/barcodeshapefactory.cpp, line 39
> > <http://git.reviewboard.kde.org/r/104235/diff/1/?file=52504#file52504line39>
> >
> >     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.

This is the tooltip text. I guess you missed that? :) The title text is "Barcode shape", set in the argument to the base class KoShapeFactoryBase(...).


- Friedrich W. H.


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


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
> 
>

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