----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120507/#review68050 -----------------------------------------------------------
I wouldn't wait for a ship it since, as always the patch is huge, I don't want to speak for others but it's really hard for anybody to make a relevant review this way. I know we had our ups and downs so take this advice as a sign that I care enough to not leave this review request unanswered. If you find the patch good just ship it. Also I support the effort of trying to improve and to avoid duplication but all I see is that 'CSVDialog' keeps getting new members and judging by their names it has become the most complex state machine I've ever seen. Here are some nitpicks I came up with by looking at the request. kmymoney/plugins/csvimport/investmentwizardpage.ui <https://git.reviewboard.kde.org/r/120507/#comment47429> Do we really need to center all strings? As a translator I found it verry hard to translate the csvimporter because of the many rich text directives. Please take KMyMoney UI as an example. kmymoney/plugins/csvimport/investprocessing.h <https://git.reviewboard.kde.org/r/120507/#comment47428> Whitespace errors (useless whitespace in empty lines or at the end of line) are pretty well highlighted by reviewboard. - Cristian Oneț On Oct. 5, 2014, 6:34 p.m., Allan Anderson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120507/ > ----------------------------------------------------------- > > (Updated Oct. 5, 2014, 6:34 p.m.) > > > Review request for KMymoney. > > > Repository: kmymoney > > > Description > ------- > > Improve functionality across distros. > It was also the intention to save the windows settings, but it was realised > that there was a liklihood that the next file to be imported could have quite > different column width and number of rows. So, it was decided to display the > whole of the file being imported instead. > Also,the UI has been improved to avoid displaying split rows, with all > possible rows being displayed, the window height aligning with the rows, and > the width displaying complete columns, as far as the screen allowed. > Further, I reworked some areas of the code to avoid duplication and improve > re-usability. > > > Diffs > ----- > > kmymoney/plugins/csvimport/lines-datewizardpage.ui 884f3ba > kmymoney/plugins/csvimport/separatorwizardpage.ui b6dba9f > kmymoney/plugins/csvimport/bankingwizardpage.ui 0d6a4a8 > kmymoney/plugins/csvimport/csvdialog.h 24abd9a > kmymoney/plugins/csvimport/csvdialog.cpp ab20ed4 > kmymoney/plugins/csvimport/csvdialog.ui 61ab9ce > kmymoney/plugins/csvimport/csvimporterplugin.h d2d2f6c > kmymoney/plugins/csvimport/csvimporterplugin.cpp 602b4d9 > kmymoney/plugins/csvimport/introwizardpage.ui a597d56 > kmymoney/plugins/csvimport/investmentdlg.cpp 5e7b266 > kmymoney/plugins/csvimport/investmentwizardpage.ui 846836e > kmymoney/plugins/csvimport/investprocessing.h fa8ffa0 > kmymoney/plugins/csvimport/investprocessing.cpp 1629e14 > > Diff: https://git.reviewboard.kde.org/r/120507/diff/ > > > Testing > ------- > > Tested with numerous banking and investment files from different sources, on > Linux Mint KDE and Ubuntu Unity. > > > Thanks, > > Allan Anderson > >
_______________________________________________ KMyMoney-devel mailing list KMyMoney-devel@kde.org https://mail.kde.org/mailman/listinfo/kmymoney-devel
