Subject: Re: [PATCH v6 2/3] replay: make atomic ref updates the default behavior

From: Junio C Hamano <[email-masked]>
Date: Fri, 31 Oct 2025 12:59:56 -0700

Elijah Newren <[email-masked]> writes:

> I'm not sure the implementation details section above makes sense to
> include in the commit message; it feels like it's not providing much
> high level information nor much "why" information, but just presenting
> an alternative view of the information people will find in the patch.
> Perhaps leave it out?

Sounds like a good thing to do.

>> Test suite changes:
>>
>> All existing tests that expected command output now use
>> --ref-action=print to preserve their original behavior. This keeps
>> the tests valid while allowing them to verify that the pipeline workflow
>> still works correctly.
>>
>> New tests were added to verify:
>>   - Default atomic behavior (no output, refs updated directly)
>>   - Bare repository support (server-side use case)
>>   - Equivalence between traditional pipeline and atomic updates
>>   - Real atomicity using a lock file to verify all-or-nothing guarantee
>>   - Test isolation using test_when_finished to clean up state
>>
>> The bare repository tests were fixed to rebuild their expectations
>> independently rather than comparing to previous test output, improving
>> test reliability and isolation.
>
> The above paragraph sounds like you are comparing to an earlier
> series, which will confuse future readers who only compare to code
> that existed before your patches.

Yup, such an update relative to previous iterations belongs in the
cover letter and below the three-dash line.

> Otherwise, the patch looks good.  This is really close to being ready
> to merge; just a few minor fixups needed that I highlighted above.

Yup, I agree with all the comments I saw here.  Thanks for a great
review.