On Thu, Jun 7, 2018 at 10:34 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Wed, Jun 6, 2018 at 3:16 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: >> On Wed, May 30, 2018 at 10:03 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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
