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

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

kristofferhaugsbakk@fastmail.com writes:

quoted

From: Kristoffer Haugsbakk <redacted>

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.

perhaps?

quoted

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

Signed-off-by: Kristoffer Haugsbakk <redacted>
---

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.