Re: [RFC PATCH 2/2] rebase: support --trailer
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- May 16, 2025, 10:04 UTC
- 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:
Show 16 quoted lines
> 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