Re: [PATCH] scripts: checkpatch: steer people away from using file paths
From: Joe Perches <joe@perches.com>
Date: 2023-07-25 21:18:22
Also in:
workflows
On Tue, 2023-07-25 at 08:59 -0700, Jakub Kicinski wrote:
We repeatedly see noobs misuse get_maintainer by running it on the file paths rather than the patchfile. This leads to authors of changes (quoted commits and commits under Fixes) not getting CCed. These are usually the best reviewers! The file option should really not be used by noobs, unless they are just trying to find a maintainer to manually contact.
noobs is not an appropriate use IMO for a commit message.
This is what I had in mind.
<shrug> I think it's unnecessary and this content should be better described in some process doc.
quoted hunk ↗ jump to hunk
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
[]
quoted hunk ↗ jump to hunk
@@ -544,7 +546,13 @@ foreach my $file (@ARGV) { if ($from_filename && (vcs_exists() && !vcs_file_exists($file))) { warn "$P: file '$file' not found in version control $!\n"; } - if ($from_filename || ($file ne "&STDIN" && vcs_file_exists($file))) { + if ($from_filename) { + if (!$silence_file_warning) { + warn "$P: WARNING: Prefer running the script on patches as " + . "generated by git format-patch. Selecting paths is known " + . "to miss recipients!\n";
Don't separate a single output message into multiple lines. Coalesce the string elements. Also, this should show some reason why this isn't appropriate as a patch to a single file would not have this issue. e.g.: When a patch series touches multiple files, showing all maintainers is useful. see: <some process doc>
quoted hunk ↗ jump to hunk
@@ -1089,6 +1098,10 @@ version: $V --pattern-depth=0 --remove-duplicates --rolestats] Notes: + Using "-f file" is generally discouraged, running the script on a filepatch + (as generated by git format-patch) is usually the right thing to do. + Commit message is an integral part of the change and $P + will extract additional information from it (keywords, Fixes tags etc.)
-f <file> "filepatch" doesn't appear in the kernel at all. Use "patch file". grammar: The commit message is... may instead of will