----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106749/#review20108 -----------------------------------------------------------
Ship it! Otherwise it look good. I haven't tried to build the patch but I think you can safely push changes that are in the cvsimport plugin since you are the author of it. kmymoney/plugins/csvimport/csvdialog.cpp <http://git.reviewboard.kde.org/r/106749/#comment15939> Is this intentional (removing 'rs' from 'Headers')? kmymoney/plugins/csvimport/csvdialog.cpp <http://git.reviewboard.kde.org/r/106749/#comment15940> ditto to what? since 'the above' was removed... kmymoney/plugins/csvimport/csvdialog.cpp <http://git.reviewboard.kde.org/r/106749/#comment15941> If you really need these magic numbers please give them some names. kmymoney/plugins/csvimport/csvdialog.cpp <http://git.reviewboard.kde.org/r/106749/#comment15942> Can this be removed? I really don't like commented code :). - Cristian Oneț On Oct. 6, 2012, 8:11 p.m., Allan Anderson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106749/ > ----------------------------------------------------------- > > (Updated Oct. 6, 2012, 8:11 p.m.) > > > Review request for KMymoney. > > > Description > ------- > > Firstly, an apology regarding the size of this change. The initial intent > was to highlight any header or trailer rows chosen by the user to be excluded > from the import, instead of deleting them, in order to improve confidence > that all was as intended. This change was not major. However, during > testing, I began more and more to dislike the untidy way the ui appeared, > with top and/or bottom rows of the table widget possibly having their height > clipped. Unfortunately, this turned out to involve far more than I expected, > for instance when dealing with differing column widths within the same file, > with the horizontal scrollbar, when present, clipping rows. Also, when > moving between wizard pages, the pages had differing heights, and this too > affected the table appearance. > > Also, a small change was made to fix the position of the 'Back' button on the > wizard, so it wasn't shifted when other buttons appeared or disappeared. > > Then, it was discovered that there were some difficulties in dealing with > field delimiter selection changes. The best way to deal with this turned out > to be to determine the appropriate delimiter to use. > > Finally, another small change, in the investment wizard, where the validity > of numeric fields was not checked during import, but only when the decimal > symbol was selected. > > > Diffs > ----- > > kmymoney/plugins/csvimport/bankingwizardpage.ui 168a197 > kmymoney/plugins/csvimport/completionwizardpage.ui 3a44e24 > kmymoney/plugins/csvimport/csvdialog.h 7d25407 > kmymoney/plugins/csvimport/csvdialog.cpp f767b79 > kmymoney/plugins/csvimport/csvdialog.ui 6bdfb1b > kmymoney/plugins/csvimport/csvutil.h fb47100 > kmymoney/plugins/csvimport/csvutil.cpp c2ea436 > kmymoney/plugins/csvimport/introwizardpage.ui d36fd9f > kmymoney/plugins/csvimport/investmentdlg.h 406a2c9 > kmymoney/plugins/csvimport/investmentdlg.cpp 2419aeb > kmymoney/plugins/csvimport/investmentwizardpage.ui bd86e1f > kmymoney/plugins/csvimport/investprocessing.h 8aea482 > kmymoney/plugins/csvimport/investprocessing.cpp 985d302 > kmymoney/plugins/csvimport/lines-datewizardpage.ui f748f2c > kmymoney/plugins/csvimport/redefinedlg.cpp 0e3d9a0 > kmymoney/plugins/csvimport/redefinedlgdecl.ui f6847c8 > kmymoney/plugins/csvimport/separatorwizardpage.ui 85bb618 > kmymoney/plugins/csvimport/symboltabledlg.h d8ea95b > kmymoney/plugins/csvimport/symboltabledlg.cpp b047f3c > > Diff: http://git.reviewboard.kde.org/r/106749/diff/ > > > Testing > ------- > > Extensive testing with numerous data files of differing numbers of rows and > columns, and different column widths. > > > Thanks, > > Allan Anderson > >
_______________________________________________ KMyMoney-devel mailing list KMyMoney-devel@kde.org https://mail.kde.org/mailman/listinfo/kmymoney-devel