Re: [PATCH] cherry-pick: don't forget -s on failure

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 12, 2012, 22:45 UTC
Message-ID: <7v8vcec13d.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vd31qc1p3.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 6 quoted lines

> I think we had a separate topic around cherry-pick that needs the
> footer thing accessible from cherry-pick recently ($gmane/204755).
>
> I think the code movement in this patch is a good one.
>
> Thanks.

Having said that, the behaviour after this patch is applied is not quite right.

A typical .git/MERGE_MSG that is left after "cherry-pick" gives the control back to you asking for help, with your patch that adds the sign-off at the end, would look like this:

    cherry-pick: don't forget -s on failure

    In case 'git cherry-pick -s <commit>' failed, the user had to use 'git
    commit -s' (i.e. state the -s option again), which is easy to forget
    about.  Instead, write the signed-off-by line early, so plain 'git
    commit' will have the same result.

    Signed-off-by: Miklos Vajna <vmiklos@suse.cz>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

    Conflicts:
            builtin/commit.c

    Signed-off-by: Junio C Hamano <gitster@pobox.com>

Notice two issues?

 - The additional S-o-b should come immediately after the existing
   block of footers.

 - And the last entry in the existing footer block is already mine,
   so there shouldn't have been a new and duplicated one added.

I am not sure how reusable the moved function is without enhancements for your purpose. The logic to identify the footer needs to be enhanced so that an "end" pointer to point at the byte before the caller added "Conflicts: " can be given, and pretend as if it is the end of the buffer, unlike in the fresh commit case where it can consider the real end of the buffer as such.

Or something like that.