From: Junio C Hamano <gitster@pobox.com>
To: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Cc: Brandon Casey <drafnel@gmail.com>,
git@vger.kernel.org, Brandon Casey <bcasey@nvidia.com>
Subject: Re: [PATCH 00/11] alternative unify appending of sob
Date: Wed, 28 Nov 2012 21:56:45 -0800 [thread overview]
Message-ID: <7vvccprlya.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <CACsJy8AfaETfikvB+zON4iiX2HBehJg-xX2GGmKYHueX8rv3qw@mail.gmail.com> (Nguyen Thai Ngoc Duy's message of "Mon, 26 Nov 2012 14:56:49 +0700")
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> On Mon, Nov 26, 2012 at 8:45 AM, Brandon Casey <drafnel@gmail.com> 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.
next prev parent reply other threads:[~2012-11-29 5:57 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-26 1:45 [PATCH 00/11] alternative unify appending of sob Brandon Casey
2012-11-26 1:45 ` [PATCH 01/11] sequencer.c: remove broken support for rfc2822 continuation in footer Brandon Casey
2012-11-26 1:45 ` [PATCH 02/11] t/test-lib-functions.sh: allow to specify the tag name to test_commit Brandon Casey
2012-11-26 1:45 ` [PATCH 03/11] t/t3511: add some tests of 'cherry-pick -s' functionality Brandon Casey
2012-11-26 1:45 ` [PATCH 04/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer Brandon Casey
2012-11-29 0:02 ` Junio C Hamano
2012-11-29 1:28 ` Brandon Casey
2012-11-26 1:45 ` [PATCH 05/11] sequencer.c: always separate "(cherry picked from" from commit body Brandon Casey
2012-11-26 1:45 ` [PATCH 06/11] sequencer.c: teach append_signoff how to detect duplicate s-o-b Brandon Casey
2012-11-29 0:35 ` Junio C Hamano
2012-11-26 1:45 ` [PATCH 07/11] sequencer.c: teach append_signoff to avoid adding a duplicate newline Brandon Casey
2012-11-26 1:45 ` [PATCH 08/11] t4014: more tests about appending s-o-b lines Brandon Casey
2012-11-26 1:45 ` [PATCH 09/11] format-patch: stricter S-o-b detection Brandon Casey
2012-11-26 1:45 ` [PATCH 10/11] format-patch: update append_signoff prototype Brandon Casey
2012-11-26 1:45 ` [PATCH 11/11] Unify appending signoff in format-patch, commit and sequencer Brandon Casey
2012-11-26 7:56 ` [PATCH 00/11] alternative unify appending of sob Nguyen Thai Ngoc Duy
2012-11-29 5:56 ` Junio C Hamano [this message]
2012-11-26 22:00 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vvccprlya.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=bcasey@nvidia.com \
--cc=drafnel@gmail.com \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.