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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 27, 2012 at 3:30 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Martin von Zweigbergk <martin.von.zweigbergk@xxxxxxxxx> 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
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]