[PATCH] sequencer: write useful reflog message for fast-forward

[PATCH] sequencer: write useful reflog message for fast-forward

[Ramkumar Ramachandra]

The following command

  $ git cherry-pick --ff b8bb3f

writes the following uninformative message to the reflog

  cherry-pick

Improve it to

  cherry-pick: fast-forward to b8bb3f

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sequencer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index ab6f8a7..ae63ff3 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -273,12 +273,14 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
 			   int unborn)
 {
 	struct ref_lock *ref_lock;
+	struct strbuf sb = STRBUF_INIT;
 
 	read_cache();
 	if (checkout_fast_forward(from, to, 1))
 		exit(1); /* the callee should have complained already */
 	ref_lock = lock_any_ref_for_update("HEAD", unborn ? null_sha1 : from, 0);
-	return write_ref_sha1(ref_lock, to, "cherry-pick");
+	strbuf_addf(&sb, "cherry-pick: fast-forward to %s", find_unique_abbrev(to, DEFAULT_ABBREV));
+	return write_ref_sha1(ref_lock, to, sb.buf);
 }
 
 static int do_recursive_merge(struct commit *base, struct commit *next,
-- 
1.8.3.1.457.gd1408e5

Re: [PATCH] sequencer: write useful reflog message for fast-forward

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> The following command
>
>   $ git cherry-pick --ff b8bb3f
>
> writes the following uninformative message to the reflog
>
>   cherry-pick
>
> Improve it to
>
>   cherry-pick: fast-forward to b8bb3f
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---

Perhaps, but a few questions in general (i.e. not limited to the
difference the patch brings in, but because you are touching the
codepath anyway).

 - Is this limited to cherry-pick?  do_pick_commit() which is the
   caller of fast_forward_to() is called not just for cherry-pick
   but also for revert, and I do not see anything that makes sure
   that it is called only when (opts->action == REPLAY_CHERRY_PICK).

 - Do we want to abbreviate?  For that matter, why even say "to
   $commit", which can be seen in the "from to" part of the reflog
   record anyway?

In other words:

	strbuf_addf(&sb, "%s: fast-forward", action_name(opts))

might be sufficient, and when sequencer learns to handle more than
cherry-pick and revert, we won't have to remember that we need to
change this part.

>  sequencer.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index ab6f8a7..ae63ff3 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -273,12 +273,14 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
>  			   int unborn)
>  {
>  	struct ref_lock *ref_lock;
> +	struct strbuf sb = STRBUF_INIT;
>  
>  	read_cache();
>  	if (checkout_fast_forward(from, to, 1))
>  		exit(1); /* the callee should have complained already */
>  	ref_lock = lock_any_ref_for_update("HEAD", unborn ? null_sha1 : from, 0);
> -	return write_ref_sha1(ref_lock, to, "cherry-pick");
> +	strbuf_addf(&sb, "cherry-pick: fast-forward to %s", find_unique_abbrev(to, DEFAULT_ABBREV));
> +	return write_ref_sha1(ref_lock, to, sb.buf);
>  }
>  
>  static int do_recursive_merge(struct commit *base, struct commit *next,

Re: [PATCH] sequencer: write useful reflog message for fast-forward

[Ramkumar Ramachandra]

Junio C Hamano wrote:
>         strbuf_addf(&sb, "%s: fast-forward", action_name(opts))

Agreed.  Can you just fix it up locally?

Thanks.