Review Request: Move plans odt export into libs/koreport.

Jarosław Staniek staniek at kde.org
Mon May 7 14:15:27 BST 2012



> On May 6, 2012, 9:10 p.m., Jarosław Staniek wrote:
> > libs/koreport/renderer/odt/KoSimpleOdtDocument.cpp, line 160
> > <http://git.reviewboard.kde.org/r/104797/diff/1/?file=59663#file59663line160>
> >
> >     how about using kWarning()? there are more such cases...
> 
> Dag Andersen wrote:
>     Removed the (useless) kDebug with Q_ASSERT.
>     (This situation should never happen...)
>     
>     AFAICS the other case(s) is not in this patch.

I still suggest to use kWarnings everywhere. Q_ASSERT is for implementation error, so not for this case. API design suggest that calling function with any arguments should not cause to program failure.


> On May 6, 2012, 9:10 p.m., Jarosław Staniek wrote:
> > libs/koreport/renderer/odt/KoSimpleOdtCheckBox.cpp, line 60
> > <http://git.reviewboard.kde.org/r/104797/diff/1/?file=59661#file59661line60>
> >
> >     Is't 0.0pt enough?
> 
> Dag Andersen wrote:
>     I'm not sure, but:
>     This is the way words and LO formats pt values (probably for a reason), so I would like keep it this way too.
>     (Same for the following similar cases)
> 
> C. Boemann wrote:
>     uhm we do gs.addPropertyPt("fo:margin", 0);
>     
>     places where we do as you say would be maybe not bug but wrong (note the different method)
> 
> Dag Andersen wrote:
>     Ahhh, so it just ends up in the xml file in the long format.
>     Ok, I'll use the gs.addPropertyPt() instead...
>     Done.

ok


> On May 6, 2012, 9:10 p.m., Jarosław Staniek wrote:
> > libs/koreport/renderer/KoReportRendererBase.cpp, line 64
> > <http://git.reviewboard.kde.org/r/104797/diff/1/?file=59659#file59659line64>
> >
> >     Since it's a new code, I propose to get the habit of using QString::compare(..., ..., Qt::CaseInsensitive)
> 
> Dag Andersen wrote:
>     This is a modification of an existing file and in a row of similar code, so I would rather keep this style in this patch and then modufy all instances in a separate patch

ok


- Jarosław


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


On April 30, 2012, 4:31 p.m., Dag Andersen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104797/
> -----------------------------------------------------------
> 
> (Updated April 30, 2012, 4:31 p.m.)
> 
> 
> Review request for Calligra, Jarosław Staniek and Adam Pigg.
> 
> 
> Description
> -------
> 
> Changes is in branch: report_odt_danders
> 
> Move plans odt export into libs/koreport.
> 
> Retains the current odt export in koreport (as odttables) and
> adds the new as odtframes.
> 
> Patch includes modifications to Plan to enable use of both formats.
> 
> 
> Diffs
> -----
> 
>   libs/koreport/CMakeLists.txt d0737c9 
>   libs/koreport/renderer/KoReportODTRenderer2.h PRE-CREATION 
>   libs/koreport/renderer/KoReportODTRenderer2.cpp PRE-CREATION 
>   libs/koreport/renderer/KoReportRendererBase.cpp 497177d 
>   libs/koreport/renderer/odt/KoSimpleOdtCheckBox.h PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtCheckBox.cpp PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtDocument.h PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtDocument.cpp PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtImage.h PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtImage.cpp PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtLine.h PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtLine.cpp PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtPicture.h PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtPicture.cpp PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtPrimitive.h PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtPrimitive.cpp PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtTextBox.h PRE-CREATION 
>   libs/koreport/renderer/odt/KoSimpleOdtTextBox.cpp PRE-CREATION 
>   libs/koreport/renderer/odt/ReportODTRenderer.h PRE-CREATION 
>   plan/libs/ui/CMakeLists.txt 4948bd3 
>   plan/libs/ui/reports/odt/KoSimpleOdtCheckBox.h 94123e2 
>   plan/libs/ui/reports/odt/KoSimpleOdtCheckBox.cpp 9a94f5b 
>   plan/libs/ui/reports/odt/KoSimpleOdtDocument.h 65bbbee 
>   plan/libs/ui/reports/odt/KoSimpleOdtDocument.cpp 7d917ef 
>   plan/libs/ui/reports/odt/KoSimpleOdtImage.h 3c40922 
>   plan/libs/ui/reports/odt/KoSimpleOdtImage.cpp 1061664 
>   plan/libs/ui/reports/odt/KoSimpleOdtLine.h 6ea097b 
>   plan/libs/ui/reports/odt/KoSimpleOdtLine.cpp 2347117 
>   plan/libs/ui/reports/odt/KoSimpleOdtPicture.h 10c3b85 
>   plan/libs/ui/reports/odt/KoSimpleOdtPicture.cpp e0df27f 
>   plan/libs/ui/reports/odt/KoSimpleOdtPrimitive.h 7cff1c7 
>   plan/libs/ui/reports/odt/KoSimpleOdtPrimitive.cpp ddea498 
>   plan/libs/ui/reports/odt/KoSimpleOdtTextBox.h 428dff7 
>   plan/libs/ui/reports/odt/KoSimpleOdtTextBox.cpp d775689 
>   plan/libs/ui/reports/odt/ReportODTRenderer.h 3c9f149 
>   plan/libs/ui/reports/odt/ReportODTRenderer.cpp 2c95ed2 
>   plan/libs/ui/reports/report.h 8cef5be 
>   plan/libs/ui/reports/reportexportpanel.cpp 973078d 
>   plan/libs/ui/reports/reportview.h 265080a 
>   plan/libs/ui/reports/reportview.cpp dea21a6 
> 
> Diff: http://git.reviewboard.kde.org/r/104797/diff/
> 
> 
> Testing
> -------
> 
> Tested with Plan, resulting files opened with Words.
> 
> 
> Thanks,
> 
> Dag Andersen
> 
>

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


More information about the calligra-devel mailing list