list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <>
To: Johannes Schindelin <>
Cc: Junio C Hamano <>,
	Johannes Schindelin via GitGitGadget <>,
Subject: Re: [PATCH 0/1] Drop last MakeMaker reference
Date: Mon, 11 Mar 2019 16:24:41 -0400	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Fri, Mar 08, 2019 at 05:27:36PM +0100, Johannes Schindelin wrote:

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


      reply	other threads:[~2019-03-11 20:24 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-25 19:27 Johannes Schindelin via GitGitGadget
2019-02-25 19:27 ` [PATCH 1/1] mingw: drop " Johannes Schindelin via GitGitGadget
2019-02-25 20:02   ` Jonathan Nieder
2019-02-25 22:07     ` Johannes Schindelin
2019-03-03  1:19 ` [PATCH 0/1] Drop last " Junio C Hamano
2019-03-07 10:10   ` Johannes Schindelin
2019-03-08  1:41     ` Junio C Hamano
2019-03-08 16:27       ` Johannes Schindelin
2019-03-11 20:24         ` Jeff King [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \
    --subject='Re: [PATCH 0/1] Drop last MakeMaker reference' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).