On 14 Sep 2023, at 15:44, Eelco Chaudron wrote:
This patch adds WARNINGs for the subject line length and the format,
i.e., the sentence should start with a capital and end with a dot.
Acked-by: Simon Horman <[email protected]>
Signed-off-by: Eelco Chaudron <[email protected]>
Simon, let me know if you are ok with the change, and I’ll commit it.
Cheers,
Eelco
---
v3: - Fixed sed to work on BSD.
v2: - Add test cases
- Made it work on files and git patches
- Changed it from 50 chars on the summary, to 70 chars on area
and summary. This way git log --oneline will fit on a 80 char
line.
tests/checkpatch.at | 29 ++++++++++++++++++++++++++
utilities/checkpatch.py | 53
++++++++++++++++++++++++++++++++++++++---------
2 files changed, 72 insertions(+), 10 deletions(-)
diff --git a/tests/checkpatch.at b/tests/checkpatch.at
index fdcdb846e..4f6b0c7b3 100755
--- a/tests/checkpatch.at
+++ b/tests/checkpatch.at
@@ -8,7 +8,14 @@ OVS_START_SHELL_HELPERS
try_checkpatch() {
# Take the patch to test from $1. Remove an initial four-space
indent
# from it and, if it is just headers with no body, add a null
body.
+ # If it does not have a 'Subject', add a valid one.
echo "$1" | sed 's/^ //' > test.patch
+ if grep 'Subject\:' test.patch >/dev/null 2>&1; then :
+ else
+ sed -i'' -e '1i\
+Subject: Patch this is.
+' test.patch
+ fi
if grep '---' expout >/dev/null 2>&1; then :
else
printf '\n---\n' >> test.patch
@@ -560,3 +567,25 @@ try_checkpatch \
"
AT_CLEANUP
+
+AT_SETUP([checkpatch - subject])
+try_checkpatch \
+ "Author: A
+ Commit: A
+ Subject: netdev: invalid case and dot ending
+
+ Signed-off-by: A" \
+ "WARNING: The subject summary should start with a capital.
+ WARNING: The subject summary should end with a dot.
+ Subject: netdev: invalid case and dot ending"
+
+try_checkpatch \
+ "Author: A
+ Commit: A
+ Subject: netdev: This is a way to long commit summary and
therefor it should report a WARNING!
+
+ Signed-off-by: A" \
+ "WARNING: The subject, '<area>: <summary>', is over 70
characters, i.e., 85.
+ Subject: netdev: This is a way to long commit summary and
therefor it should report a WARNING!"
+
+AT_CLEANUP
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 5c4aaefb3..3f42c44f2 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -792,6 +792,36 @@ def run_file_checks(text):
check['check'](text)
+def run_subject_checks(subject, spellcheck=False):
+ warnings = False
+
+ if spellcheck and check_spelling(subject, False):
+ warnings = True
+
+ summary = subject[subject.rindex(': ') + 2:]
+ area_summary = subject[subject.index(': ') + 2:]
+ area_summary_len = len(area_summary)
+ if area_summary_len > 70:
+ print_warning("The subject, '<area>: <summary>', is over 70 "
+ "characters, i.e., %u." % area_summary_len)
+ warnings = True
+
+ if summary[0].isalpha() and summary[0].islower():
+ print_warning(
+ "The subject summary should start with a capital.")
+ warnings = True
+
+ if subject[-1] not in [".", "?", "!"]:
+ print_warning(
+ "The subject summary should end with a dot.")
+ warnings = True
+
+ if warnings:
+ print(subject)
+
+ return warnings
+
+
def ovs_checkpatch_parse(text, filename, author=None,
committer=None):
global print_file_name, total_line, checking_file, \
empty_return_check_state
@@ -812,6 +842,7 @@ def ovs_checkpatch_parse(text, filename,
author=None, committer=None):
r'^@@ ([0-9-+]+),([0-9-+]+) ([0-9-+]+),([0-9-+]+) @@')
is_author = re.compile(r'^(Author|From): (.*)$', re.I | re.M |
re.S)
is_committer = re.compile(r'^(Commit: )(.*)$', re.I | re.M |
re.S)
+ is_subject = re.compile(r'^(Subject: )(.*)$', re.I | re.M | re.S)
is_signature = re.compile(r'^(Signed-off-by: )(.*)$',
re.I | re.M | re.S)
is_co_author = re.compile(r'^(Co-authored-by: )(.*)$',
@@ -911,6 +942,8 @@ def ovs_checkpatch_parse(text, filename,
author=None, committer=None):
committer = is_committer.match(line).group(2)
elif is_author.match(line):
author = is_author.match(line).group(2)
+ elif is_subject.match(line):
+ run_subject_checks(line, spellcheck)
elif is_signature.match(line):
m = is_signature.match(line)
signatures.append(m.group(2))
@@ -1029,18 +1062,18 @@ def ovs_checkpatch_file(filename):
result = ovs_checkpatch_parse(part.get_payload(decode=False),
filename,
mail.get('Author', mail['From']),
mail['Commit'])
- if spellcheck:
- if not mail['Subject'] or not mail['Subject'].strip():
- if mail['Subject']:
- mail.replace_header('Subject', sys.argv[-1])
- else:
- mail.add_header('Subject', sys.argv[-1])
- print("Subject missing! Your provisional subject is",
- mail['Subject'])
+ if not mail['Subject'] or not mail['Subject'].strip():
+ if mail['Subject']:
+ mail.replace_header('Subject', sys.argv[-1])
+ else:
+ mail.add_header('Subject', sys.argv[-1])
+
+ print("Subject missing! Your provisional subject is",
+ mail['Subject'])
- if check_spelling(mail['Subject'], False):
- print("Subject: %s" % mail['Subject'])
+ if run_subject_checks('Subject: ' + mail['Subject'], spellcheck):
+ result = True
ovs_checkpatch_print_result()
return result
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
