Re: [RFC PATCH 2/2] rebase: support --trailer

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Li

On 16/05/2025 06:42, Li Chen wrote:
> Hi Phillip,
> 
>   ---- On Thu, 08 May 2025 22:17:17 +0800  Phillip Wood <phillip.wood123@gmail.com> wrote ---
>   >
>   > (c) Only adds the trailers on the commandline. I'm a bit confused by the
>   > various trailer config options - the man page reads to me like "git
>   > interpret-trailers" can add missing trailers that are configured but not
>   > passed on the commandline.
> 
> About part (c), just to be sure I understand correctly:
> 
> Do you want the trailer implementation to completely drop any handling of trailer configuration
> (i.e. remove parse_trailers_from_config() and related config-based behavior from the codepath and man page/documents)?
> 
> Or would you rather leave the config machinery in place, but have rebase --trailer explicitly
> ignore all trailer.* configuration and only append the exact trailers passed on its command line?

I think I had misunderstood what trailer.ifMissing did. I was concerned 
that it could add trailers that were not on the command-line but I don't 
think that's the case. We certainly want to respect the config for 
trailer.<alias>.key and trailer.<alias>.command as they make it possible 
for the user to set "trailer.review.key=Reviewed-by" and 
"trailer.review.command=git var GIT_COMMITTER_IDENT | sed 's/[^>]*$//' 
#" and then run "git rebase --trailer=review" to add their Reviewed-by: 
trailer. I think it makes sense to respect the other config as well - 
that does mean that trailers that the user passes on the command-line 
may not be added because they already exist or are configured not to be 
added if they are missing but is consistent with "git 
interpret-trailers". It means that the user can set 
"trailer.myKey.ifExists=doNothing" and then run "git rebase --trailer 
MyKey=value" to ensure all the commits have a MyKey trailer without 
duplicating it in the commits where it already exists.

That's a long-winded way of saying that on reflection I think respecting 
the trailer config setting is the right thing to do after all.

Best Wishes

Phillip

> Please let me know which you prefer (or if there’s a third path I’m missing) and I’ll add the patches accordingly.
> 
> Regards,
> Li