[PATCH RFC 2/2] docs: process: submitting-patches: clarify imperative mood suggestion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
---
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





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux