Re: [PATCH v3 1/2] format-patch: simplify get_notes_arg parameters

From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
Date: Oct 2, 2026, 18:51 UTC
Message-ID: <d2360e73-6602-4418-af2e-265054ba8e2b@app.fastmail.com>
In-Reply-To: <xmqqtsn4xd17.fsf@gitster.g>

On Fri, Oct 2, 2026, at 18:50, Junio C Hamano wrote:

Show 21 quoted lines

> kristofferhaugsbakk@fastmail.com writes:
>
>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>
>> 85bd88a7 (revision: add rdiff_log_arg to rev_info, 2025-09-25) added
>> `rdiff_log_arg` to `struct rev_info`. I changed `get_notes_arg` by
>> simply replacing the first argument with an access on this struct
>> member. But the second argument was already `struct rev_info`. So I
>> should have just simplified to *only* passing that parameter. Let’s do
>> that now.
>
> The readers do not necessarily want to read the "author's journey"
> narrative in log messages.  Let's be more detached and objective,
> like
>
>   85bd88a7e8 (revision: add rdiff_log_arg to rev_info, 2025-09-25)
>   updated get_notes_args() to push into rev->rdiff_log_arg instead
>   of an explicit strvec, but left the rev argument as the second
>   parameter and strvec *arg as the first. Simplify the signature of
>   get_notes_args() to take only struct rev_info *rev, dropping the
>   redundant strvec *arg parameter.

I don’t get what objective improvement there is by replacing “I did” with “it happened”. This is not a gratuitous incidental biography but just says what your alternative says, only with a personal pronoun, less technical diction, and one word longer.

But I think we can shorten it with a little show-don’t-tell:

    85bd88a7 (revision: add rdiff_log_arg to rev_info, 2025-09-25) added
    `rdiff_log_arg` to `struct rev_info`. `get_notes_arg` was changed to
    take a second parameter, namely that member:

        get_notes_args(&(rev.rdiff_log_arg), &rev);

    But this is obviously unnecessary; we can just use `&rev`.

    Now is also a good time to format this `for_each...` line since it’s
    gotten quite long.

That’s 16 words less than my first version.

Show 16 quoted lines

>
> perhaps?
>
>> Now is also a good time to format this `for_each...` line since it’s
>> gotten quite long.
>>
>> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
>> ---
>>
>> Notes (testing):
>>     just compile tested
>
> The code change looks good.  As long as this stays as a static helper
> function, this is not a loss of flexibility but a simplification of
> the calling convention.
>

Thanks for reviewing.