From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
Date: Tue, 03 Mar 2026 20:36:53 GMT
Subject: Re: [PATCH v7 5/5] rebase: support --trailer
Message-ID: <22e1de8e-935d-4efa-9fa8-ef8d9b4ffc6a@app.fastmail.com>
In-Reply-To: <824809c3-72ac-43fb-8a93-4f48e0727e6a@gmail.com>

On Tue, Mar 3, 2026, at 16:05, Phillip Wood wrote:
>>[snip]
>> diff --git a/sequencer.c b/sequencer.c
>> index a3eb39bb25..a60c2a0cde 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> [...]
>> @@ -2025,6 +2027,9 @@ static int append_squash_message(struct strbuf *buf, const char *body,
>>   		if (opts->signoff)
>>   			append_signoff(buf, 0, 0);
>>
>> +		if (opts->trailer_args.nr)
>> +			amend_strbuf_with_trailers(buf, &opts->trailer_args);
>
> I wonder if it would be better to add the trailers before the signoff so
> that "git rebase --signoff --trailer='Reviewed-by: ...'" adds the
> "Reviewed-by:" trailer before the "Signed-off-by:" trailer.

Why is that? Is that because that is the practice in this project (and
maybe others)?

I would expect it to act like however `--trailer` already acts on
git-commit(1) and git-tag(1). I would have to test that.

In any case these `--signoff` options are considered a historical
mistake now (since they special-case one key).

The logic for before/after and so on are supposed to be handled by the
trailer config, it seems. But last I looked that was only for same-key
trailers and duplicates. Not for logic like keeping your own signoff
last.

>[snip]