From: Siddharth Asthana <siddharthasthana31@gmail.com>
Date: Sat, 08 Nov 2025 13:23:38 GMT
Subject: Re: [PATCH v7 0/3] replay: make atomic ref updates the default
Message-ID: <00a5a8f3-f761-46e8-84cc-4bd95db68b49@gmail.com>
In-Reply-To: <906fba13-fc84-411c-a43f-baaa2b90ed95@gmail.com>


On 07/11/25 21:18, Phillip Wood wrote:
> Hi Siddharth
>
> On 05/11/2025 19:15, Siddharth Asthana wrote:
>
>>      @@ builtin/replay.c: int cmd_replay(int argc,
>>            determine_replay_mode(repo, &revs.cmdline, onto_name, 
>> &advance_name,
>>                          &onto, &update_refs);
>>             ++    /* Build reflog message */
>>      ++    if (advance_name_opt)
>>      ++        strbuf_addf(&reflog_msg, "replay --advance %s", 
>> advance_name_opt);
>

Hi Phillip,


> This appends the name of the branch being advanced, rather than what's 
> being picked. As this message is written to the reflog of the branch 
> that's being advanced adding the branch name to the message is kind of 
> redundant but we can always change this later when we have more 
> experience with "--ref-action"


You are absolutely right about the redundancy. I went with the branch 
name to match what users typed on the command line, but since it's in 
that branch's own reflog, just "replay --advance" might be cleaner.

Happy to adjust this in a follow-up if the current approach proves 
confusing in practice.


>
>>      ++    else
>>      ++        strbuf_addf(&reflog_msg, "replay --onto %s",
>>      ++                oid_to_hex(&onto->object.oid));
>
> This looks good.
>
> Thanks for working on this, I think this is probably ready to me merged.


Thank you for all the detailed feedback throughout this series - it 
really helped improve the implementation!

Siddharth


>
> Phillip
>