{"thread":{"id":"34204","subject":"[PATCH] sequencer: write useful reflog message for fast-forward","startedAt":"2013-06-18T18:35:18Z","lastAt":"2013-06-18T19:02:56Z","messageCount":3,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"221280","messageId":"1371580518-32455-1-git-send-email-artagnon@gmail.com","threadId":"34204","inReplyTo":null,"subject":"[PATCH] sequencer: write useful reflog message for fast-forward","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-18T18:35:18Z","receivedAt":"2013-06-18T18:35:18Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The following command\n\n  $ git cherry-pick --ff b8bb3f\n\nwrites the following uninformative message to the reflog\n\n  cherry-pick\n\nImprove it to\n\n  cherry-pick: fast-forward to b8bb3f\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n sequencer.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex ab6f8a7..ae63ff3 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -273,12 +273,14 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \t\t\t   int unborn)\n {\n \tstruct ref_lock *ref_lock;\n+\tstruct strbuf sb = STRBUF_INIT;\n \n \tread_cache();\n \tif (checkout_fast_forward(from, to, 1))\n \t\texit(1); /* the callee should have complained already */\n \tref_lock = lock_any_ref_for_update(\"HEAD\", unborn ? null_sha1 : from, 0);\n-\treturn write_ref_sha1(ref_lock, to, \"cherry-pick\");\n+\tstrbuf_addf(&sb, \"cherry-pick: fast-forward to %s\", find_unique_abbrev(to, DEFAULT_ABBREV));\n+\treturn write_ref_sha1(ref_lock, to, sb.buf);\n }\n \n static int do_recursive_merge(struct commit *base, struct commit *next,\n-- \n1.8.3.1.457.gd1408e5\n"},{"id":"221283","messageId":"7vzjun45dr.fsf@alter.siamese.dyndns.org","threadId":"34204","inReplyTo":"1371580518-32455-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] sequencer: write useful reflog message for fast-forward","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-18T18:53:52Z","receivedAt":"2013-06-18T18:53:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> The following command\n>\n>   $ git cherry-pick --ff b8bb3f\n>\n> writes the following uninformative message to the reflog\n>\n>   cherry-pick\n>\n> Improve it to\n>\n>   cherry-pick: fast-forward to b8bb3f\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n\nPerhaps, but a few questions in general (i.e. not limited to the\ndifference the patch brings in, but because you are touching the\ncodepath anyway).\n\n - Is this limited to cherry-pick?  do_pick_commit() which is the\n   caller of fast_forward_to() is called not just for cherry-pick\n   but also for revert, and I do not see anything that makes sure\n   that it is called only when (opts->action == REPLAY_CHERRY_PICK).\n\n - Do we want to abbreviate?  For that matter, why even say \"to\n   $commit\", which can be seen in the \"from to\" part of the reflog\n   record anyway?\n\nIn other words:\n\n\tstrbuf_addf(&sb, \"%s: fast-forward\", action_name(opts))\n\nmight be sufficient, and when sequencer learns to handle more than\ncherry-pick and revert, we won't have to remember that we need to\nchange this part.\n\n>  sequencer.c | 4 +++-\n>  1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index ab6f8a7..ae63ff3 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -273,12 +273,14 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n>  \t\t\t   int unborn)\n>  {\n>  \tstruct ref_lock *ref_lock;\n> +\tstruct strbuf sb = STRBUF_INIT;\n>  \n>  \tread_cache();\n>  \tif (checkout_fast_forward(from, to, 1))\n>  \t\texit(1); /* the callee should have complained already */\n>  \tref_lock = lock_any_ref_for_update(\"HEAD\", unborn ? null_sha1 : from, 0);\n> -\treturn write_ref_sha1(ref_lock, to, \"cherry-pick\");\n> +\tstrbuf_addf(&sb, \"cherry-pick: fast-forward to %s\", find_unique_abbrev(to, DEFAULT_ABBREV));\n> +\treturn write_ref_sha1(ref_lock, to, sb.buf);\n>  }\n>  \n>  static int do_recursive_merge(struct commit *base, struct commit *next,\n"},{"id":"221292","messageId":"CALkWK0kU01zfvyietjuTFk01UteT+uuKXfjrfSirn=i8FHCxCw@mail.gmail.com","threadId":"34204","inReplyTo":"7vzjun45dr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] sequencer: write useful reflog message for fast-forward","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-18T19:02:56Z","receivedAt":"2013-06-18T19:02:56Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n>         strbuf_addf(&sb, \"%s: fast-forward\", action_name(opts))\n\nAgreed.  Can you just fix it up locally?\n\nThanks.\n"}]}