> On April 19, 2013, 11:54 a.m., Cristian Oneț wrote: > > csvexportdlgdecl.ui is not in the patch. Please provide the full patch > > against git master not only of your latest commit: > > $git format-patch origin/master > > Allan Anderson wrote: > It definitely shows as being in the first patch, on which the second > patch is based. When I do > git format-patch origin/master -o ~/ , I get the two patch files as in > this review. - > > /home/aga/0001-BUG-317614-Add-CSV-export-capability.patch > /home/aga/0002-BUG-317614-Add-CSV-export-capability-minor-changes-f.patch > > ~/Git/kmymoney $ git log > commit ad1cabdd80298588a4cbeec19689f5e0e738139d > Author: Allan Anderson <agande...@gmail.com> > Date: Fri Apr 19 12:25:10 2013 +0100 > > BUG:317614 - Add CSV export capability - minor changes following > Reviewboard. > > commit 45a3330ab990b9f6c7839ed2ad487ee0fed8299a > Author: Allan Anderson <agande...@gmail.com> > Date: Sat Mar 30 18:10:09 2013 +0000 > > BUG:317614 - Add CSV export capability > > Does not that look correct? > > > Cristian Oneț wrote: > Although combining the two patches produces the correct result it's > pretty hard to make a review based on it. > > Cristian Oneț wrote: > You could have used 'git commit --amend' when committing the minor > changes to update your previous commit (you can do that for commits that were > not yet pushed). > > Alvaro Soliverez wrote: > You can also add multiple commits in one patch > > git format-patch -2 > file.patch > > This will create a patch with the 2 latest commits > > Allan Anderson wrote: > I have previously used 'git commit --amend' to produce a single new > patch. I actually did an additional patch for the update to make it easier > for you to see the changes I'd made! Apologies if it's made life harder. > > I'll now do a combined patch as per Alvaro. Erm, that produces a folder > containing the two patches, which, I don't think, is going to make assessment > any easier. > > > Marko Käning wrote: > Your latest patch installs and runs fine. > > Cristian Oneț wrote: > git format-patch -2 --stdout > file.patch > > Marko Käning wrote: > Hi Cristian, am I missing something here? I thought the current Diff_r3 > incorporates already everything needed... > > Allan Anderson wrote: > I'm not sure if I'm expected to respond here, or whether the action is > with Cristian, when he is free, but I don't want everyone to be waiting for > everyone else. Also, I'm not trying to push, as I understand fully that the > real world can get in the way. > So, as Marko indicated, there is a third, combined patch here. > Also, Cristian's command does indeed produce a single, total patch, as > was required. > Finally, Cristian said (April 19, 2013, 11:06 a.m.) > "Please update the patch in this review before shipping it." > Having done that, was that the go ahead to ship?
Only now I understood why the "--stdout" is needed. OK, I guess you should wait for Cristian's response, though. - Marko ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109803/#review31282 ----------------------------------------------------------- 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 > >
_______________________________________________ KMyMoney-devel mailing list KMyMoney-devel@kde.org https://mail.kde.org/mailman/listinfo/kmymoney-devel
