From: Martin von Zweigbergk Date: Sat, 28 Jul 2012 04:52:59 GMT Subject: Re: [PATCH 2/2] log: remove redundant check for merge commit Message-ID: In-Reply-To: <7vfw8czvj5.fsf@alter.siamese.dyndns.org> On Fri, Jul 27, 2012 at 3:30 PM, Junio C Hamano wrote: > Martin von Zweigbergk writes: >> Also re-initializing rev_info fields to the same values already set in >> init_revisions(). Oops, that should have been " _avoid_ re-initializing". > I suspect that > explicit initialization to revs.ignore_merges outself can be called > belt-and-braces defensive programming to avoid getting surprised by > future changes to what init_revisions() would do, so I do not have > strong preference either way. I suspected the same, but OTOH, if it was ever changed, one would have to go through all call sites anyway. I checked the other calls to init_revisions (48 of them, I think) and I found only 3 other places where a field was re-initialized to the same value. In this case it was done inconsistently even within the file. Seeing init_revisions() followed by "revs.ignore_merges = 1;" in one place but not the other can easily lead one to believe that the other place does not ignore merges. Do you want a reroll with updated commit messages (the missing "avoid" above, the dropped "seems like" about the prefix in 1/2)? Since you don't have a strong preference about the explicit initialization, I assume I can leave those hunks in. I would clarify my reasoning, though. Martin