[Kmymoney-devel] Review Request 109803: Add CSV export capability.

[Allan Anderson]

> On April 21, 2013, 1:39 p.m., Thomas Baumgart wrote:
> > I just did a more formal review and noticed a few things that might receive some improvements. Sorry for being late with the review.

Appreciate you finding the time, thanks.  A couple of responses now, then I'll go through the rest.


> On April 21, 2013, 1:39 p.m., Thomas Baumgart wrote:
> > kmymoney/plugins/csvexport/csvexportdlg.h, line 55
> > <http://git.reviewboard.kde.org/r/109803/diff/3/?file=139975#file139975line55>
> >
> >     Using the xxxDecl classes is somewhat old fashioned. No need for that in this case. You can directly apply the multi inheritance to CsvExportDialog and don't need CsvExportDlgDecl at all. See http://apidocs.meego.com/1.2/qt4/designer-using-a-ui-file.html for a good tutorial. Personally, I am currently using the single inheritance method in another project, but that does not mean anything.

And I only started to use that to be more compatible with the project!


> On April 21, 2013, 1:39 p.m., Thomas Baumgart wrote:
> > kmymoney/plugins/csvexport/csvwriter.cpp, line 218
> > <http://git.reviewboard.kde.org/r/109803/diff/3/?file=139981#file139981line218>
> >
> >     Such constructs are the reason for so many performance problems (though not here I assume): why don't you simply write
> >     
> >       str += QLatin1String("R,");
> >

I picked up on QStringBuilder recently and had the impression it improved performance.  I'm not saying what I've done is correct, and I bow to your experience.


- Allan


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


On April 19, 2013, 5:52 p.m., Allan Anderson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109803/
> -----------------------------------------------------------
> 
> (Updated April 19, 2013, 5:52 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Description
> -------
> 
> Add CSV export capability.  Modify existing KMyMoney File menu in order to make menu item positions more logical.
> 
> 
> This addresses bug 317614.
>     http://bugs.kde.org/show_bug.cgi?id=317614
> 
> 
> Diffs
> -----
> 
>   kmymoney/plugins/CMakeLists.txt 81ca458 
>   kmymoney/plugins/csvexport/CMakeLists.txt PRE-CREATION 
>   kmymoney/plugins/csvexport/csvexportdlg.h PRE-CREATION 
>   kmymoney/plugins/csvexport/csvexportdlg.cpp PRE-CREATION 
>   kmymoney/plugins/csvexport/csvexportdlgdecl.ui PRE-CREATION 
>   kmymoney/plugins/csvexport/csvexporterplugin.h PRE-CREATION 
>   kmymoney/plugins/csvexport/csvexporterplugin.cpp PRE-CREATION 
>   kmymoney/plugins/csvexport/csvwriter.h PRE-CREATION 
>   kmymoney/plugins/csvexport/csvwriter.cpp PRE-CREATION 
>   kmymoney/plugins/csvexport/kmm_csvexport.desktop PRE-CREATION 
>   kmymoney/plugins/csvexport/kmm_csvexport.rc PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/109803/diff/
> 
> 
> Testing
> -------
> 
> Exported numerous checking and investment CSV files, and then imported into KMyMoney via CSV import (discovering a few minor issues in the existing KMyMoney accounts in the process.)
> Ran astyle and Krazy2.
> 
> 
> Thanks,
> 
> Allan Anderson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20130421/d50c5523/attachment.html>