-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4462/#review6307
-----------------------------------------------------------
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/convdate.cpp
<http://reviewboard.kde.org/r/4462/#comment6000>
use character assignment as in QString slash = '/'; Krazy will warn you anyway.
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/convdate.cpp
<http://reviewboard.kde.org/r/4462/#comment6001>
What is so special about -1 to be detected and yet allow -2? The documentation of the method does not include a hint. Better use a range check.
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.h
<http://reviewboard.kde.org/r/4462/#comment6002>
No need to keep a version for each file. SVN will take good care about that for you. If you want to keep a version for the whole package, add it as a #define in a header. (This might apply to the other files as well but I just comment on it here).
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.h
<http://reviewboard.kde.org/r/4462/#comment6004>
Where is this referenced in this header file? If it's unused, please remove.
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.h
<http://reviewboard.kde.org/r/4462/#comment6003>
Indentation: please try to run astyle-kmymoney.sh now and then. (I know, I forget it too sometimes)
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.cpp
<http://reviewboard.kde.org/r/4462/#comment6006>
Please use KMessageBox.
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.cpp
<http://reviewboard.kde.org/r/4462/#comment6010>
This would not work on my box as I don't have "/home/aga". You might want to take a look at KStandardDirs. It has plenty of methods to get the right name for your file.
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.cpp
<http://reviewboard.kde.org/r/4462/#comment6014>
This whole line is a NOOP. inFile is just contructed in the source line above and thus cannot be open.
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.cpp
<http://reviewboard.kde.org/r/4462/#comment6013>
Please try to use layouts instead of fixed sizes.
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlg.cpp
<http://reviewboard.kde.org/r/4462/#comment6019>
I spotted this also in another place: why do you have a fixed (limited) list of codecs? There's an example at http://doc.trolltech.com/4.4/tools-codecs-mainwindow-cpp.html in the method MainWindow::findCodecs()
on how to extract them from Qt itself.
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlg.cpp
<http://reviewboard.kde.org/r/4462/#comment6022>
Use the QString::clear() as in trInvData.memo.clear() to clear strings.
trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/redefinedlg.cpp
<http://reviewboard.kde.org/r/4462/#comment6023>
Use "const QString& type" instead of "QString type". Applies to other locations as well.
- Thomas
On 2010-06-27 03:49:22, Cristian Onet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4462/
> -----------------------------------------------------------
>
> (Updated 2010-06-27 03:49:22)
>
>
> Review request for kmymoney.
>
>
> Summary
> -------
>
> Allan's csvimporter plugin
>
>
> Diffs
> -----
>
> trunk/extragear/office/kmymoney/kmymoney/plugins/CMakeLists.txt 1143143
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/CMakeLists.txt PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/convdate.h PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/convdate.cpp PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvdatetest.h PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvdatetest.cpp PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.h PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.cpp PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.ui PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterplugin.h PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterplugin.cpp PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlg.h PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlg.cpp PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlg.ui PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/kmm_csvimport.desktop PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/kmm_csvimport.rc PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/redefinedlg.h PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/redefinedlg.cpp PRE-CREATION
> trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/redefinedlg.ui PRE-CREATION
>
> Diff: http://reviewboard.kde.org/r/4462/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Cristian
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kmymoney-devel/attachments/20100628/2d42d6c7/attachment-0001.htm