On Thu, Nov 13, 2008 at 12:41:15PM +0000, Charles Bailey wrote: > This option stops git mergetool from aborting at the first failed merge. > This allows some additional use patterns. Merge conflicts can now be > previewed one at time and merges can also be skipped so that they can be > performed in a later pass. > > There is also a mergetool.keepGoing configuration variable covering the > same behaviour. I think this series addresses the questions I remember from the first posting, and it looks sane overall to me. My only two complaints are: > Documentation/config.txt | 4 +++ > Documentation/git-mergetool.txt | 12 +++++++++- > git-mergetool.sh | 46 ++++++++++++++++++++++++++++++-------- > 3 files changed, 51 insertions(+), 11 deletions(-) This patch in particular changes some of the innards of mergetool, and while the changes look good to me via reading, I would feel even more comfortable if there were some tests (it looks like your previous t7610 gives at least a basic sanity check, but it might be nice to test to make sure we detect merge failures, too). > - return > + return 0 > [...] > - return > + return 0 > [...] > - exit 1 > + return 1 > [...] > - exit 1 > + return 1 > [...] > - exit 1 > + cleanup_temp_files > + return 1 One of these is not like the others. Is there a reason to add a cleanup_temp_files here (and if so, should it be noted in the commit message, and/or be in a separate commit?). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
