Message-Id: <20241220-submitting-patches-imperative-v1-2-ee874c1859b3@pengutronix.de>
Date: Fri, 20 Dec 2024 10:09:35 +0100
From: Ahmad Fatoum <a.fatoum@...gutronix.de>
To: Jonathan Corbet <corbet@....net>
Cc: workflows@...r.kernel.org, linux-doc@...r.kernel.org, 
 linux-kernel@...r.kernel.org, Borislav Petkov <bp@...e.de>, 
 Rob Herring <robh@...nel.org>, Frank Li <Frank.li@....com>, 
 kernel@...gutronix.de, Ahmad Fatoum <a.fatoum@...gutronix.de>
Subject: [PATCH RFC 2/2] docs: process: submitting-patches: clarify
 imperative mood suggestion

While we expect commit message titles to use the imperative mood,
it's ok for commit message bodies to first include a blurb describing
the background of the patch, before delving into what's being done
to address the situation.

Make this clearer by adding a clarification after the imperative mood
suggestion as well as listing Rob Herring's commit 52bb69be6790
("dt-bindings: ata: pata-common: Add missing additionalProperties on
child nodes") as a good example commit message.

Signed-off-by: Ahmad Fatoum <a.fatoum@...gutronix.de>
---
I liked Rob's commit message, because:

  - It has multiple subsystem prefixes
  - Uses imperative mood in the commit message title
  - Doesn't use it in the commit message body showing that it's
    not a hard requirement
  - Is short and gives a succinct background
  - Explains not only why to do the change, but also why it's ok
    to do it

Admittedly though, a C example may be easier to grok for a general
audience. I can search for one if that's preferred (or maybe someone
has a suitable commit already they wish to suggest?)
---
 Documentation/process/submitting-patches.rst | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 1474c84b3ac562f5806d85e8ef5b6e9d23572e59..b10534e460aa30f2751208bd1eca242841ba5edb 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -96,6 +96,11 @@ instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
 to do frotz", as if you are giving orders to the codebase to change
 its behaviour.
 
+The goal of the imperative mood is to make especially commit message
+titles (the :ref:`patch_subject_line`) succinct and to the point.
+The explanation body should be more detailed and take care to explain
+the background motivating the change. (see :ref:`patch_explanation_body`).
+
 If you want to refer to a specific commit, don't just refer to the
 SHA-1 ID of the commit. Please also include the oneline summary of
 the commit, to make it easier for reviewers to know what it is about.
@@ -610,6 +615,8 @@ that, if you have your patches stored in a ``git`` repository, proper patch
 formatting can be had with ``git format-patch``.  The tools cannot create
 the necessary text, though, so read the instructions below anyway.
 
+.. _patch_subject_line:
+
 Subject Line
 ^^^^^^^^^^^^
 
@@ -699,6 +706,8 @@ patch in the permanent changelog.  If the ``from`` line is missing,
 then the ``From:`` line from the email header will be used to determine
 the patch author in the changelog.
 
+.. _patch_explanation_body:
+
 Explanation Body
 ^^^^^^^^^^^^^^^^
 
@@ -717,6 +726,15 @@ _all_ of the compile failures; just enough that it is likely that
 someone searching for the patch can find it. As in the ``summary
 phrase``, it is important to be both succinct as well as descriptive.
 
+Here is one example of a good commit message::
+
+  dt-bindings: ata: pata-common: Add missing additionalProperties on child nodes
+
+  The PATA child node schema is missing constraints to prevent unknown
+  properties. As none of the users of this common binding extend the child
+  nodes with additional properties, adding "additionalProperties: false"
+  here is sufficient.
+
 .. _backtraces:
 
 Backtraces in commit messages

-- 
2.39.5

Powered by blists - more mailing lists