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

2012-09-13 Thread Junio C Hamano
Miklos Vajna  writes:

> +void append_signoff(struct strbuf *msgbuf, int ignore_footer)
> +{
> + struct strbuf sob = STRBUF_INIT;
> + int i;
> +
> + strbuf_addstr(&sob, sign_off_header);
> + strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
> + getenv("GIT_COMMITTER_EMAIL")));
> + strbuf_addch(&sob, '\n');
> + for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] 
> != '\n'; i--)
> + ; /* do nothing */
> + if (prefixcmp(msgbuf->buf + i, sob.buf)) {
> + if (!i || !ends_rfc2822_footer(msgbuf))
> + strbuf_addch(msgbuf, '\n');
> + strbuf_addbuf(msgbuf, &sob);
> + }
> + strbuf_release(&sob);
> +}

Hrm, what is this thing trying to do?  It does start scanning from
the end (ignoring the "Conflicts:" thing) to see who the last person
that signed it off was, but once it decides that it needs to add a
new sign-off, it still adds it at the very end anyway.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-09-13 Thread Junio C Hamano
Miklos Vajna  writes:

> In case 'git cherry-pick -s ' 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.
>
> Also update 'git commit -s', so that in case there is already a relevant
> Signed-off-by line before the Conflicts: line, it won't add one more at
> the end of the message.
>
> Signed-off-by: Miklos Vajna 
> ---
>
> On Wed, Sep 12, 2012 at 03:45:10PM -0700, Junio C Hamano  
> wrote:
>>  - The additional S-o-b should come immediately after the existing
>>block of footers.
>
> This was trivial to fix.

Indeed.  Just inserting before starting to add "Oh, there were
conflicts, and add the info on them" before doing it at the end is
all it takes.  Simple and straightforward---I like it.

> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -149,6 +149,12 @@ test_commit () {
>   notick=yes
>   shift
>   fi &&
> + signoff= &&
> + if test "z$1" = "z--signoff"
> + then
> + signoff="$1"
> + shift
> + fi &&
>   file=${2:-"$1.t"} &&
>   echo "${3-$1}" > "$file" &&
>   git add "$file" &&

This is somewhat iffy.  Shouldn't "test_commit --signoff --notick" work?

> @@ -156,7 +162,7 @@ test_commit () {
>   then
>   test_tick
>   fi &&
> - git commit -m "$1" &&
> + git commit $signoff -m "$1" &&
>   git tag "$1"
>  }

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html