> On Oct. 7, 2014, 12:47 p.m., Cristian Oneț wrote: > > 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. > > Allan Anderson wrote: > Thanks Cristian. As you say, the patch is huge, although that mainly > comes from several (nearly all) UI files having changes. Alvaro mentioned > recently that he doesn't use QTDesigner because of its tendency to bloat the > files, I suspect rather like Windows Office. If I were more confident, I too > would use a text editor. In addition, some code I wanted to change was also > going to need to be reused, so I extracted it into two separate routines, and > the original large routine I was able to dispense with. I don't foresee any > further major changes in the plugin! > This patch and the recent associated "BUG:339044 REVIEW:120260 - Fix CSV > import wizard not working correctly on Windows", between them removed 431 > lines, so I think I've moved in the right direction.
Committed. remote: This commit is available for viewing at: remote: http://commits.kde.org/kmymoney/50f7b6c47ad4f326b7cd6b2e0f8746b8391b3810 > On Oct. 7, 2014, 12:47 p.m., Cristian Oneț wrote: > > kmymoney/plugins/csvimport/investmentwizardpage.ui, line 302 > > <https://git.reviewboard.kde.org/r/120507/diff/1/?file=316711#file316711line302> > > > > 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. > > Allan Anderson wrote: > You did mention this before, but now I understand to what you are > referring. May be this is another example of Designer bloat. I might be > able to get a simpler result from producing these UI labels via code. I'll > keep my eye on that. The UI heading labels have been trimmed. They are still cemtered, but no longer HTML-based - Allan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120507/#review68050 ----------------------------------------------------------- On Oct. 16, 2014, 12:15 p.m., Allan Anderson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120507/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2014, 12:15 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
