From: Phillip Wood Date: Fri, 16 May 2025 10:04:37 GMT Subject: Re: [RFC PATCH 2/2] rebase: support --trailer Message-ID: <8b431310-ff03-424b-b24c-672d9b492057@gmail.com> In-Reply-To: <196d79d922d.127d7eb7f870411.3309775784450179401@linux.beauty> Hi Li On 16/05/2025 06:42, Li Chen wrote: > Hi Phillip, > > ---- On Thu, 08 May 2025 22:17:17 +0800 Phillip Wood 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..key and trailer..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