Re: [RFC PATCH 0/5] format-patch: automate cover letter range-diff

[Duy Nguyen <pclouds@gmail.com>]

On Thu, Jun 7, 2018 at 10:34 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Wed, Jun 6, 2018 at 3:16 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Wed, May 30, 2018 at 10:03 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> Dscho recently implemented a 'tbdiff' replacement as a Git builtin named
>>> git-branch-diff[1] which computes differences between two versions of a
>>> patch series. Such a diff can be a useful aid for reviewers when
>>> inserted into a cover letter. However, doing so requires manual
>>> generation (invoking git-branch-diff) and copy/pasting the result into
>>> the cover letter.
>>
>> Another option which I wanted to go is delegate part of cover letter
>> generation to a hook (or just a config key that contains a shell
>> command). This way it's easier to customize cover letters. We could
>> still have a good fallback that does shortlog, diffstat and tbdiff.
>
> It is common on this mailing list to turn down requests for new hooks
> when the requested functionality could just as easily be implemented
> via a wrapper script. So, my knee-jerk reaction is that a hook to
> customize the cover letter may be overkill when the same functionality
> could likely be implemented relatively easily by a shell script which
> invokes git-format-patch and customizes the cover letter
> after-the-fact. Same argument regarding a config key holding a shell
> command. But, perhaps there are cases which don't occur to me which
> could be helped by a config variable or such.

I think format-patch --cover-letter nowadays does more stuff that's
not so easy to simply rewrite it in a shell script. My original
problem with format-patch is it hard codes shortlog settings and you
can't list patches with patch number (e.g. "[1/2] foo bar"). The
simplest way is let format-patch does it stuff as usual and
"outsource" some cover letter's body generation to a script.

But it's ok. I could try to code the patch numbering thing in
format-patch and maybe submit a patch or two for that later.

> Of course, by the same reasoning, the --range-diff functionality
> implemented by this patch series, which is just a convenience, could
> be handled by a wrapper script, thus is not strictly needed. On the
> other hand, given that interdiffs and range-diffs are so regularly
> used in re-rolls on this list (and perhaps other mailing list-based
> projects) may be argument enough in favor of having such an option
> built into git-format-patch.
-- 
Duy