checkpatch.pl: Add check for Change-Id
PATCH 1/1 checkpatch.pl: Add check for Change-Id
A commit hook for the Gerrit code review server inserts change identifiers so Gerrit can track patches through multiple revisions. These identifiers are noise in the context of the upstream kernel. (Many Gerrit servers are private. Even given a public instance, given only a Change-Id, one must guess which server a change was tracked on. Patches submitted to the Linux kernel mailing lists should be able to stand on their own. If it's truly useful to reference code review on a Gerrit server, a URL is a much clearer way to do so.) Thus, issue an error when a Change-Id: line is encountered.
--- scripts/checkpatch.pl | 6 ++++++ 1 file changed, 6 insertions(+)
scripts/checkpatch.pl +6 -0
@@ -1891,6 +1891,12 @@ sub process { } } +# Check for unwanted Gerrit info + if ($line =~ /^\s*change-id:/i) { + ERROR("BAD_SIGN_OFF", + "Remove Gerrit Change-Id's before submitting upstream.\n" . $herecurr); + } + # Check for wrappage within a valid hunk of the file if ($realcnt != 0 && $line !~ m{^(?:\+|-| |\\ No newline|$)}) { ERROR("CORRUPTED_PATCH",
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Joe Perches
maintainerQuoted 10 lines · Christopher Covington
A commit hook for the Gerrit code review server inserts change identifiers so Gerrit can track patches through multiple revisions. These identifiers are noise in the context of the upstream kernel. (Many Gerrit servers are private. Even given a public instance, given only a Change-Id, one must guess which server a change was tracked on. Patches submitted to the Linux kernel mailing lists should be able to stand on their own. If it's truly useful to reference code review on a Gerrit server, a URL is a much clearer way to do so.) Thus, issue an error when a Change-Id: line is encountered.
[]
scripts/checkpatch.pl
Quoted 1 line
scripts/checkpatch.pl +0
-0
[]
scripts/checkpatch.pl sub process { · line 1899
Quoted 10 lines
scripts/checkpatch.pl +6
-0@@ -1891,6 +1891,12 @@ sub process { } } +# Check for unwanted Gerrit info + if ($line =~ /^\s*change-id:/i) { + ERROR("BAD_SIGN_OFF", + "Remove Gerrit Change-Id's before submitting upstream.\n" . $herecurr); + } +
I think this needs an "$in_commit_log" test added
if ($in_commit_log &&
$line =~ /^\s*change-id:/i) {And maybe use a separate "TYPE", not "BAD_SIGN_OFF". Maybe "GERRIT_CHANGE_ID"
Hi Joe,
Quoted 27 lines · Joe Perches
Quoted 10 lines · Christopher Covington
A commit hook for the Gerrit code review server inserts change identifiers so Gerrit can track patches through multiple revisions. These identifiers are noise in the context of the upstream kernel. (Many Gerrit servers are private. Even given a public instance, given only a Change-Id, one must guess which server a change was tracked on. Patches submitted to the Linux kernel mailing lists should be able to stand on their own. If it's truly useful to reference code review on a Gerrit server, a URL is a much clearer way to do so.) Thus, issue an error when a Change-Id: line is encountered.
[]
scripts/checkpatch.pl
Quoted 1 line
scripts/checkpatch.pl +0
-0[]
scripts/checkpatch.pl sub process { · line 1899
Quoted 10 lines
scripts/checkpatch.pl +6
-0@@ -1891,6 +1891,12 @@ sub process { } } +# Check for unwanted Gerrit info + if ($line =~ /^\s*change-id:/i) { + ERROR("BAD_SIGN_OFF", + "Remove Gerrit Change-Id's before submitting upstream.\n" . $herecurr); + } +I think this needs an "$in_commit_log" test added
Quoted 2 lines
if ($in_commit_log &&
$line =~ /^\s*change-id:/i) {This would mean that Change-Id lines that follow a Signed-off-by line will not get caught. I expect that the common case is to use the commit hook shipped with Gerrit [1], which inserts the Change-Id above the Signed-off-by, so the proposed change is fine by me.
Quoted 3 lines
And maybe use a separate "TYPE", not "
BAD_SIGN_OFF". Maybe "GERRIT_CHANGE_ID"
Will do.
Thanks,
Christopher
Signature · 3 lines
Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation.
PATCH v2 1/1 checkpatch.pl: Add check for Change-Id
A commit hook for the Gerrit code review server [1] inserts change identifiers so Gerrit can track patches through multiple revisions. These identifiers are noise in the context of the upstream kernel. (Many Gerrit servers are private. Even given a public instance, given only a Change-Id, one must guess which server a change was tracked on. Patches submitted to the Linux kernel mailing lists should be able to stand on their own. If it's truly useful to reference code review on a Gerrit server, a URL is a much clearer way to do so.) Thus, issue an error when a Change-Id line is encountered before the Signed-off-by.
--- scripts/checkpatch.pl | 6 ++++++ 1 file changed, 6 insertions(+)
scripts/checkpatch.pl +6 -0
@@ -1891,6 +1891,12 @@ sub process { } } +# Check for unwanted Gerrit info + if ($in_commit_log && $line =~ /^\s*change-id:/i) { + ERROR("GERRIT_CHANGE_ID", + "Remove Gerrit Change-Id's before submitting upstream.\n" . $herecurr); + } + # Check for wrappage within a valid hunk of the file if ($realcnt != 0 && $line !~ m{^(?:\+|-| |\\ No newline|$)}) { ERROR("CORRUPTED_PATCH",
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by the Linux Foundation.