Re: [PATCH v6 0/4] rebase: support --trailer

[Junio C Hamano <gitster@pobox.com>]

Li Chen <me@linux.beauty> writes:

> From: Li Chen <chenl311@chinatelecom.cn>
>
> This series routes all trailer insertion through an in-process path, removing
> the fork/exec to builtin/interpret-trailers and tempfile juggling. 

This description makes it sound as if the code before this patch
series drove "interpret-trailers" via fork/exec and tempfile
juggling.  And that contradicts the title of the topic, "rebase:
support --trailer", which implies that the topic is about the "git
rebase" command, and that "git rebase" before this patch series did
not support trailers, not even with fork/exec and tempfile juggling.

Which is it?

I see trailer.c:amend_file_with_trailers() does fork out to the
"git interpret-trailers" command and is called by "git commit" and
"git tag".  Perhaps you are updating the amend_file_with_trailers()
helper function to do the in-process thing, so that "git commit" and
"git tag" no longer needs fork/exec and tempfile juggling?  

That would be great, regardless of "rebase", and if you used that
updated helper function to teach "rebase" to deal with trailers
in-process, that would be wonderful.

If the main part of the series (i.e. [1/4]-[4/4]) needs rerolling,
could you be a bit more careful when writing the cover letter to
make it easier for even those who are seeing this series for the
first time to understand what is going on?

> The first
> three commits centralize logic to reduce overhead and simplify error handling.

... in what code paths?  "In command X and Y where they do Z", "All
the call flows that lead to helper function F by eliminating the
need to do G that is costly and replacing it with H", etc., is what
I would expect to see in such a description.

> The final commit adds git rebase --trailer, currently supported
> with the merge backend only (rejecting apply-only scenarios and
> validating input early).

Sounds sensible.
Li Chen <me@linux.beauty> writes: