Re: [PATCH 2/2] log: remove redundant check for merge commit

From: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Date: Jul 28, 2012, 04:52 UTC
Message-ID: <CAOeW2eFROgZ+taqFvCHoSTVrQZnaQZo567e=73BcA+d5uSjKDA@mail.gmail.com>
In-Reply-To: <7vfw8czvj5.fsf@alter.siamese.dyndns.org>

On Fri, Jul 27, 2012 at 3:30 PM, Junio C Hamano <gitster@pobox.com> wrote:

> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
>> Also re-initializing rev_info fields to the same values already set in
>> init_revisions().

Oops, that should have been " _avoid_ re-initializing".

Show 5 quoted lines

> 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