git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 0/1] Drop last MakeMaker reference

From
Jeff King <peff@peff.net>
Date
Mar 11, 2019, 20:24 UTC
Message-ID
<20190311202441.GB18263@sigill.intra.peff.net>
In-Reply-To
<nycvar.QRO.7.76.6.1903081718330.41@tvgsbejvaqbjf.bet>
On Fri, Mar 08, 2019 at 05:27:36PM +0100, Johannes Schindelin wrote:
Show 7 quoted lines
> > Or perhaps GGG can learn to avoid 0/1 for a single-patch topic ;-)
> 
> It is easier, and more consistent, to have a cover letter even then, for
> things like the broader explanation of the context, the changes since the
> previous iteration, and the range-diff (which would make v2, v3, v4, etc
> utterly unreadable from my point of view if they were integrated into the
> single patches, as I used to do with interdiffs).
Just my two cents:

As a reviewer, I generally prefer not to see a separate cover letter for a single patch. At least for the first version (I agree it gets unwieldy showing a range-diff after the "---" for subsequent versions, unless it happens to be pretty short).

My reasoning is that I've noticed that many of the GGG-sent patches often end up duplicating quite a bit of content between the cover letter and the commit message of the patch (or worse, putting things only in the cover letter that really could go into the commit message). That doubles the time I spend reading and understanding the patch's rationale (and I'm speaking subjectively here, of course; I didn't measure it).

I don't think it's an _inherent_ problem with a separate cover letter. But something about the workflow causes people to write up over-long cover letters. Which presumably is the fact that they're presented with a PR textbox to write the rationale separately from when they wrote the commit message. So they err on the side of repeating themselves, and never see the two pieces "together" (like the reader will), which makes the redundancy more obvious.

I'd say 99% of the time a single-patch doesn't need any cover letter material at all. And even a multi-patch series really just needs a tl;dr of the problem and a sketch of the solution. In both cases, the commit messages should carry the meat.

(That's all specific to our project, of course; I know many projects don't care about commit messages at all and expect PR descriptions to be the first-class explanations).

-Peff
Previous: Johannes Schindelin
Message 9 of 9 in “Drop last MakeMaker reference”
  1. 0/1 Drop last MakeMaker referenceJohannes Schindelin via GitGitGadget, Feb 25, 2019
  2. 1/1 mingw: drop MakeMaker referenceJohannes Schindelin via GitGitGadget, Feb 25, 2019
  3. Jonathan NiederFeb 25, 2019
  4. Johannes SchindelinFeb 25, 2019
  5. Junio C HamanoMar 3, 2019
  6. Johannes SchindelinMar 7, 2019
  7. Junio C HamanoMar 8, 2019
  8. Johannes SchindelinMar 8, 2019
  9. Jeff KingMar 11, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.