LKML Archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Bagas Sanjaya <bagasdotme@gmail.com>,
	Franziska Naepelt <franziska.naepelt@googlemail.com>,
	keyrings@vger.kernel.org, dhowells@redhat.com,
	dwmw2@infradead.org, linux-kernel@vger.kernel.org,
	Franziska Naepelt <franziska.naepelt@gmail.com>,
	kernel test robot <lkp@intel.com>,
	Linux SPDX Licenses <linux-spdx@vger.kernel.org>,
	Linux Kernel Janitors <kernel-janitors@vger.kernel.org>
Subject: Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues
Date: Tue, 6 Jun 2023 18:25:24 +0300	[thread overview]
Message-ID: <e44d03cf-9993-483c-b3d4-6185f5c028cc@kadam.mountain> (raw)
In-Reply-To: <CT5NH4XXIYQF.5XXJE6JA5FZP@suppilovahvero>

On Tue, Jun 06, 2023 at 05:51:09PM +0300, Jarkko Sakkinen wrote:
> On Tue Jun 6, 2023 at 4:38 PM EEST, Dan Carpenter wrote:
> > On Tue, Jun 06, 2023 at 07:28:52PM +0700, Bagas Sanjaya wrote:
> > > On Fri, Jun 02, 2023 at 10:59:02AM +0200, Franziska Naepelt wrote:
> > > > The following issues are fixed:
> > > > - WARNING: Missing or malformed SPDX-License-Identifier tag
> > > > - ERROR: trailing statements should be on next line
> > > > - WARNING: braces {} are not necessary for single statement blocks
> > > > - ERROR: space required before the open parenthesis '('
> > > > - ERROR: code indent should use tabs where possible
> > > > - WARNING: please, no spaces at the start of a line
> > > > - WARNING: Missing a blank line after declarations
> > > 
> > > Again, write the patch description in imperative mood (e.g. "Do foo").
> > > 
> >
> > Why do you care about imperative tense?  Imperative tense doesn't
> > matter.  What matters is that you can understand the issue and how it
> > looks like to the user.  I was working with a group of foreign students
> > and it was painful to see the contortions that they went through to make
> > a commit message imperative.  It's like saying "Bake a cake", "Ok, now
> > bake it while juggling."  The cake ends up worse.  And the commit
> > message end up worse when we force nonsense rules like this.
> 
> How about a simple and stupid reason?
> 
> Usually I write commit message without caring about this. Then I rewrite
> the commit message and 9/10 it gets shorter. Based on empirical
> experience, imperative form has minimum amount of extra words.
> 

I'm looking through the git log to see if it's true the imperative tense
commit message are shorter and better and neither one of those things is
obvious to me.

This patch had an imperative subject already so it was already kind of
imperative.  Does every sentence have to be imperative or can you just
add a "Fix it." to the end?

I don't want to belittle the challenges you face around the English
language but I think students were less fluent than you are.  So maybe
imperative tense works for you but it definitely made their commit
messages far worse.

regards,
dan carpenter


  reply	other threads:[~2023-06-06 15:25 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-01 19:05 [PATCH] certs/extract-cert: Fix checkpatch issues Franziska Naepelt
2023-06-02  2:27 ` kernel test robot
2023-06-02  3:09 ` kernel test robot
2023-06-02  8:59 ` [PATCH v2] " Franziska Naepelt
2023-06-06 12:28   ` Bagas Sanjaya
2023-06-06 13:38     ` Dan Carpenter
2023-06-06 14:51       ` Jarkko Sakkinen
2023-06-06 15:25         ` Dan Carpenter [this message]
2023-06-06 16:03           ` Jarkko Sakkinen
2023-06-06 17:59             ` Franziska Näpelt
2023-06-06 18:29               ` Dan Carpenter
2023-06-06 21:43           ` Ben Boeckel
2023-06-07 16:11             ` Jarkko Sakkinen
2023-06-09 14:01         ` Dan Carpenter
2023-06-09 15:37           ` Jarkko Sakkinen
2023-06-09 15:43             ` Jarkko Sakkinen
2023-06-06 12:38   ` Jarkko Sakkinen
2023-06-06 12:44     ` Jarkko Sakkinen
2023-06-06 12:47       ` Jarkko Sakkinen
2023-06-06 14:44   ` David Howells

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=e44d03cf-9993-483c-b3d4-6185f5c028cc@kadam.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=bagasdotme@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=franziska.naepelt@gmail.com \
    --cc=franziska.naepelt@googlemail.com \
    --cc=jarkko@kernel.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spdx@vger.kernel.org \
    --cc=lkp@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).