linux-kernel - Re: [PATCH v3] checkpatch: warn about novice phrases in commit messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7d938f5514c8e4705cab01e776648fd2b6be0edd.camel@perches.com>
Date: Thu, 24 Jul 2025 00:04:07 -0700
From: Joe Perches <joe@...ches.com>
To: Ignacio Peña <ignacio.pena87@...il.com>, Andy
 Whitcroft <apw@...onical.com>
Cc: Dwaipayan Ray <dwaipayanray1@...il.com>, Lukas Bulwahn
	 <lukas.bulwahn@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] checkpatch: warn about novice phrases in commit
 messages

On Wed, 2025-07-23 at 23:28 -0400, Ignacio Peña wrote:
> Add detection for common phrases that make patches appear less
> confident. These phrases are often used by newcomers and can make
> their contributions seem less professional or uncertain.
> 
> The regex uses qr{} syntax as suggested for better readability and
> potential pre-compilation benefits.

Unneessary paragraph.

> 
> Examples of detected phrases:
> - 'please apply/merge/consider/review'
> - 'hope this helps'
> - 'my first patch/contribution'
> - 'newbie/beginner here'
> - 'not sure if (this is) correct'
> - 'sorry if/for'
> 
> This helps newcomers learn the expected communication style in
> kernel development, where direct and confident communication is
> preferred.
> 
> Link: https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> Suggested-by: Joe Perches <joe@...ches.com>

I did not suggest this.
I am merely trying to improve the patch and its readability.
I do not need to have any credit for this.

> Signed-off-by: Ignacio Peña <ignacio.pena87@...il.com>
> ---
> Changes in v3:
> - Use qr{} syntax instead of // for the regex (Joe Perches)
> - Remove comment about the suggestion (Joe Perches)
> - Drop the SHA enforcement patch based on maintainer feedback

This last line should be part of a cover letter to the patch set
rather than added to this specific patch.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3266,6 +3266,14 @@ sub process {
>  			     "A patch subject line should describe the change not the tool that found it\n" . $herecurr);
>  		}
>  
> +# Check for novice phrases in commit message
> +		if ($in_commit_log && !$non_utf8_charset) {

You still haven't answered why $!non_utf8_charset is useful.

> +			if ($line =~ qr{\b(?:please\s+(?:apply|merge|consider|review)|hope\s+this\s+helps|my\s+first\s+(?:patch|contribution)|(?:newbie|beginner)\s+here|not\s+sure\s+if\s+(?:this\s+is\s+)?correct|sorry\s+(?:if|for))\b}i) {

And this is still not human readable.
I much prefer describing each phrase on a separate line like:

			my $novice_phrases = qr{(?xi:
				please\s+(?:apply|merge|consider|review) |
				hope\s+this\s+helps |
				my\s+first\s+(?:patch|contribution) |
				(?:newbie|beginner)\s+here |
				not\s+sure\s+if\s+(?:this\s+is\s+)?correct |
				sorry\s+(?:if|for)
			)};

			if ($line =~ /\b$novice_phrase\b/) {