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

Sorry, I meant to CC the list. See below.

On Sat, Jul 28, 2012 at 9:56 PM, Martin von Zweigbergk
[off-list ref] wrote:

On Fri, Jul 27, 2012 at 11:52 PM, Junio C Hamano [off-list ref] wrote:

quoted

Junio C Hamano [off-list ref] writes:

It seems to have some interaction with your other topic, though.
These two patches alone will pass the existing tests, but merging it
with mz/rebase-range breaks t3412.  I didn't look into it, but
perhaps this breaks "git cherry" in some way?

Yes, it breaks "git cherry" quite badly, by not ignoring merges at
all. I incorrectly assumed that ignore_merges was about revision
traversal, but now I think it's only diff output from 'git log' (and
possibly others). What I think tricked me was seeing that
"ignore_merges = 1" closely followed by a comment saying "ignore
merges". But now I think the explicit code to "ignore merges" is
necessary (as show by the failing test case), but can be replaced by
"rev_info.max_parents = 1". Setting "ignore_merges = 1", OTOH, now
seems doubly redundant: not only does it set the same value as was set
in init_revisions, but it's also irrelevant. Since cherry doesn't
generate any diff output, I think ignore_merges is never used.
Flipping the values of all of "ignore_merges", "combine_merges" and
"diff" does not have any effect on test cases at least. I hope my
explanation makes some sense at least...

I'll send a reroll when I get time.