diff mbox series

[v3,1/3] SubmittingPatches: write problem statement in the log in the present tense

Message ID 20220127190259.2470753-2-gitster@pobox.com (mailing list archive)
State Accepted
Commit fa1101afb66dfb3ad325ad135b7bed59e4067dd5
Headers show
Series [v3,1/3] SubmittingPatches: write problem statement in the log in the present tense | expand

Commit Message

Junio C Hamano Jan. 27, 2022, 7:02 p.m. UTC
We give a guidance for proposed log message to write problem
statement first, followed by the reasoning behind, and recipe for,
the solution.  Clarify that we describe the situation _before_ the
proposed patch is applied in the present tense (not in the past
tense e.g. "we used to do X, but thanks to this commit we now do Y")
for consistency.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/SubmittingPatches | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Emily Shaffer March 3, 2022, 11:59 p.m. UTC | #1
On Thu, Jan 27, 2022 at 11:02:57AM -0800, Junio C Hamano wrote:
> 
> We give a guidance for proposed log message to write problem
> statement first, followed by the reasoning behind, and recipe for,
> the solution.  Clarify that we describe the situation _before_ the
> proposed patch is applied in the present tense (not in the past
> tense e.g. "we used to do X, but thanks to this commit we now do Y")
> for consistency.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/SubmittingPatches | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 92b80d94d4..7225a0fb52 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -142,6 +142,13 @@ The body should provide a meaningful commit message, which:
>  
>  . alternate solutions considered but discarded, if any.
>  
> +[[present-tense]]
> +The problem statement that describes the status quo is written in the
> +present tense.  Write "The code does X when it is given input Y",
> +instead of "The code used to do Y when given input X".  You do not
> +have to say "Currently"---the status quo in the problem statement is
> +about the code _without_ your change, by project convention.
> +

The writing sample helps a lot here. "Present tense" is a poor example,
but I think it's common for native language speakers to know less about
specific grammar words (like "imperative mood" below), even though they
can often instinctively tell what form you're describing when seeing a
sample.

As for the norm itself, I think it is a good one, and I've seen it
pointed out in code reviews frequently. Thanks.

Reviewed-by: Emily Shaffer <emilyshaffer@google.com>

>  [[imperative-mood]]
>  Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
>  instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
> -- 
> 2.35.0-177-g7d269f5170
>
Junio C Hamano March 4, 2022, 12:23 a.m. UTC | #2
Emily Shaffer <emilyshaffer@google.com> writes:

> As for the norm itself, I think it is a good one, and I've seen it
> pointed out in code reviews frequently. Thanks.
>
> Reviewed-by: Emily Shaffer <emilyshaffer@google.com>

Thanks.

Was this meant to be sent long after the topic was merged to
'master' at 83760938 (Merge branch 'jc/doc-log-messages',
2022-02-11)?
Emily Shaffer March 4, 2022, 11:41 p.m. UTC | #3
On Thu, Mar 03, 2022 at 04:23:45PM -0800, Junio C Hamano wrote:
> 
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > As for the norm itself, I think it is a good one, and I've seen it
> > pointed out in code reviews frequently. Thanks.
> >
> > Reviewed-by: Emily Shaffer <emilyshaffer@google.com>
> 
> Thanks.
> 
> Was this meant to be sent long after the topic was merged to
> 'master' at 83760938 (Merge branch 'jc/doc-log-messages',
> 2022-02-11)?

As mentioned in
https://lore.kernel.org/git/YiFZicz69mDyFzXB%40google.com - no, it
wasn't. In fact, in conversation at $dayjob someone said "whatever
happened to Junio planning to write updates to SubmittingPatches?" and
someone else said "it didn't really get any review" - so I said "okay, I
will do review" instead of checking whether you merged anyway. So mostly
useless noise :)

 - Emily
diff mbox series

Patch

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 92b80d94d4..7225a0fb52 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -142,6 +142,13 @@  The body should provide a meaningful commit message, which:
 
 . alternate solutions considered but discarded, if any.
 
+[[present-tense]]
+The problem statement that describes the status quo is written in the
+present tense.  Write "The code does X when it is given input Y",
+instead of "The code used to do Y when given input X".  You do not
+have to say "Currently"---the status quo in the problem statement is
+about the code _without_ your change, by project convention.
+
 [[imperative-mood]]
 Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
 instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy