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

From: Siddharth Asthana <siddharthasthana31@gmail.com>
Date: Oct 28, 2025, 20:18 UTC
Message-ID: <90b3f359-6b24-4858-848a-531478b21f65@gmail.com>
In-Reply-To: <xmqqbjlwqq6d.fsf@gitster.g>

On 24/10/25 20:53, Junio C Hamano wrote:

Show 16 quoted lines

> Christian Couder <christian.couder@gmail.com> writes:
>
>> On Wed, Oct 22, 2025 at 8:51 PM Siddharth Asthana
>> <siddharthasthana31@gmail.com> wrote:
>>
>>> -       const char * const replay_usage[] = {
>>> +       const char *const replay_usage[] = {
>> Nit: Not sure this change is worth it, but I understand that it might
>> help pass some automated/CI tests, so not a big issue.
> I think this formatting issue came up recently on another discussion
> thread.  We found that the prevalent style in the codebase is that
> an asterisk in between tokens neither of which is variable has space
> on both sides (i.e. the preimage of the above change), so unless
> there is a specific reason to make the above change, I'd rather not
> to see such "reformatting" thrown into a patch that implements a
> feature or fixes a bug (iow, not a "clean-up styles" patch).

You are absolutely right, I will revert this formatting change in v5. The `const char * const` spacing follows the established codebase style and there's no reason to change it in a feature patch.

>
> By the way, I would be suprised if that the reason were a CI test.
> How would the preimage have been passing the same test if that is
> the case?

Good point, it wasn't a CI issue, just an unnecessary style change on my part.

Thanks, Siddharth