Re: [RFC PATCH 1/2] rebase, am: add --reviewby option

Hi Junio,

Thank you for the feedback. My current understanding is as below, please correct me if I am mistaken

---- On Wed, 07 May 2025 02:07:46 +0800  Junio C Hamano <gitster@pobox.com> wrote --- 
 > Li Chen <me@linux.beauty> writes:
 > 
 > > From: Li Chen <chenl311@chinatelecom.cn>
 > >
 > > Introduce a new `--reviewby` flag to `git rebase` and `git am` that appends a
 > 
 > Shouldn't this (and possibly the other one, I didn't look at the
 > patch text) be using the internal machinery used by interpret-trailers
 > so that we can do the usual "do not duplicated existing ones",
 > "append only at the last one is different" etc.?
 
At the moment, git-interpret‑trailers only works on a file or on
stdin. During an interactive/merge rebase we change it on the fly
without format-patch && am, which  is very convenient.

The new --trailer option already re‑uses amend_file_with_trailers() from trailer.c, so there is very little trailer‑specific code of my own.

Right now --reviewby mirrors the existing --signoff implementation, and append_signoff() itself does not use the trailer library. If you are open to keeping a dedicated --reviewby, I can send a follow‑up (or roll it into this series) that moves both sign‑off and review‑by to the common trailer helpers.

 > I also do not think we want to see one option (like the above) for
 > each trailer elements (like "Reviewed-by") that is commonly used,
 > which would lead us to adding "--helped-by", "--acked-by", etc. to
 > complement this one.
 > 

Some projects require every commit to carry a Reviewed-by: line for accountability, much like the kernel requires Signed-off-by:. A first‑class option keeps that workflow “out of the box”; otherwise people need to define an alias such as

[alias]
    rbr = rebase --trailer "Reviewed-by: $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"

which is functional but less convenient.

I would appreciate your further thoughts on whether a dedicated flag(--reviewby) is acceptable, or whether we should drop it and rely solely on the generic --trailer interface.

Thanks again for the review.

Regards, Li