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@pengutronix.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

Ahmad Fatoum <a.fatoum@pengutronix.de> writes:

> 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@pengutronix.de>

I'm rather less convinced about this one.  We already have a whole
section on describing changes.  Given that this crucial document is
already long and hard enough to get through, I don't really think that
adding some duplicate information - and the noise of more labels - is
going to improve things.

Thanks,

jon

Hello Jon,

On 30.12.24 19:40, Jonathan Corbet wrote:
> Ahmad Fatoum <a.fatoum@pengutronix.de> writes:
> 
>> 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@pengutronix.de>
> 
> I'm rather less convinced about this one.  We already have a whole
> section on describing changes.  Given that this crucial document is
> already long and hard enough to get through, I don't really think that
> adding some duplicate information - and the noise of more labels - is
> going to improve things.

Do you agree with the content of the patch in principle?

My changes were motivated by a disagreement about the necessity of having
to use the imperative mood throughout as I described in my cover letter,
so I still think think that a clarification is appropriate.

Would a v2 without the example at the end be acceptable?

Thanks,
Ahmad




> 
> Thanks,
> 
> jon
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Ahmad Fatoum <a.fatoum@pengutronix.de> writes:

> Hello Jon,
>
> On 30.12.24 19:40, Jonathan Corbet wrote:
>> Ahmad Fatoum <a.fatoum@pengutronix.de> writes:
>> 
>>> 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@pengutronix.de>
>> 
>> I'm rather less convinced about this one.  We already have a whole
>> section on describing changes.  Given that this crucial document is
>> already long and hard enough to get through, I don't really think that
>> adding some duplicate information - and the noise of more labels - is
>> going to improve things.
>
> Do you agree with the content of the patch in principle?
>
> My changes were motivated by a disagreement about the necessity of having
> to use the imperative mood throughout as I described in my cover letter,
> so I still think think that a clarification is appropriate.
>
> Would a v2 without the example at the end be acceptable?

I will always consider a patch, but the example isn't the concern,
really.  The information you are trying to add to an already too-long
document is already present there; I think that repeating it, and making
this crucial document that much more unapproachable, would actively make
things worse.

Thanks,

jon

Hello Jon,

On 06.01.25 15:57, Jonathan Corbet wrote:
> Ahmad Fatoum <a.fatoum@pengutronix.de> writes:
> 
>> Hello Jon,
>>
>> On 30.12.24 19:40, Jonathan Corbet wrote:
>>> Ahmad Fatoum <a.fatoum@pengutronix.de> writes:
>>>
>>>> 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@pengutronix.de>
>>>
>>> I'm rather less convinced about this one.  We already have a whole
>>> section on describing changes.  Given that this crucial document is
>>> already long and hard enough to get through, I don't really think that
>>> adding some duplicate information - and the noise of more labels - is
>>> going to improve things.
>>
>> Do you agree with the content of the patch in principle?
>>
>> My changes were motivated by a disagreement about the necessity of having
>> to use the imperative mood throughout as I described in my cover letter,
>> so I still think think that a clarification is appropriate.
>>
>> Would a v2 without the example at the end be acceptable?
> 
> I will always consider a patch, but the example isn't the concern,
> really.  The information you are trying to add to an already too-long
> document is already present there; I think that repeating it, and making
> this crucial document that much more unapproachable, would actively make
> things worse.

Ok, thanks for the prompt response.

Cheers,
Ahmad

> 
> Thanks,
> 
> jon
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |