Subject: Re: [PATCH 00/11] alternative unify appending of sob

From: Junio C Hamano <[email-masked]>
Date: Wed, 28 Nov 2012 21:56:45 -0800

Nguyen Thai Ngoc Duy <[email-masked]> writes:

> On Mon, Nov 26, 2012 at 8:45 AM, Brandon Casey <[email-masked]> wrote:
>
>> I've integrated Duy's series with a few minor tweaks.  I added a couple
>> of additional tests to t4014 and corrected one of the tests which had
>> incorrect behavior.  I think his sign-off's should still be valid, so I
>> kept them in.  Sorry that I've been slow, and now the two of us are stepping
>> on each other's toes, but Duy please take a look and let me know if there's
>> anything you dislike.
>
> I'm still not sure whether format-patch should follow cherry-pick's
> rule in appending sob. If it does, it probably makes more sense to fix
> the sequencer.c code then delete log-tree.c (not fixes on log-tree.c
> then delete it). I see that your changes pass all the new tests I
> added in format-patch so sequencer.c is probably good enough,
> log-tree.c changes are probably not needed. Feel free take over the
> series :-)

After reading the series over, I agree with the above.

Patch #9 that fixes the copy in log-tree.c only to discard it in
patch #11 does not seem to be the best organization of the series.
Instead, perhaps we can salvage the tests in patch #9 (but mark
failing ones as expecting failure) without updating the one in
log-tree.c, adjust prototype in patch #10 (still broken in
log-tree.c) to avoid having to make changes to the callers in patch
#11, and then conclude the series with #11?

Other than the code in patches #06 and #07 that I already commented
on, i.e. assignments in if () condition that make it harder to
follow the logic, I did not find anything majorly objectionable in
the series.

Thanks for a pleasant read.