Hi Junio, On Wed, May 13, 2015 at 10:27 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > It is not unusual when the change is trivially correct. > > I do not think this hurts, but I do not think that this is vastly > better, either. If you suspect that the previous one may fail but > you at the same time are so trusting that the one before that one > would succeed, yes, this will help in that case. But if you suspect > the previous one may fail, the one before that may have also failed, > in which case this may not be sufficient (the result of fetching may > not match what this test expects to see). > > It all depends on where our paranoia ends. The current code is very > much trusting all previous ones equally, and accepts "upon the first > error, all bets are off for the later ones". With this patch, it > becomes slightly less trusting. > > If everything else were equal, I would say this change is a "Meh" to > me, Yeah, thinking about it, it's a "meh" to me too. While this patch will improve consistency in the attempt to recover from failure, I don't think it will be useful in practice, and it's also not relevant to the goal of this series. Will drop this patch. > but I think the change improves this test in a different way. > > It begins with "Run 'git rebase --abort', just in case"; which is a > signal that it does consider that the previous one may have failed > and attempts to prepare for that possibility, while trusting the one > before that would have succeeded. And under that assumption, what > it currently does is _not_ consistent; the previous "pull --rebase" > may have failed in the "rebase" phase, in which case "abort just in > case" is a good measure to go back to the clean state, but it may > have failed in the "fetch" phase, in which case "abort" does not > help. And this patch is needed to fix that inconsistency. > > If justified in that way in the log message, then I wouldn't have > said "I do not think that this is vastly better", I think. > Thanks, Paul -- 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