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

From: Duy Nguyen <pclouds@gmail.com>
Date: Jun 7, 2018, 15:09 UTC
Message-ID: <CACsJy8D696DiWLGJgfqECBDJ9rMbpOy5bCe_knTttmR82QPCJA@mail.gmail.com>
In-Reply-To: <CAPig+cS1M3U0T7yd2hKv4Uv1NMub20bTy13-D-6drpJLJjNNAA@mail.gmail.com>

On Thu, Jun 7, 2018 at 10:34 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 23 quoted lines

> 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.

Show 7 quoted lines

> 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