Re: [ovs-dev] [PATCH v3] checkpatch: Improve accuracy and specificity of sign-off checking.
On Mon, Aug 13, 2018 at 01:16:25PM -0400, Aaron Conole wrote: > Ben Pfaff writes: > > > On Sun, Aug 12, 2018 at 07:43:14AM -0700, Ben Pfaff wrote: > >> On Sat, Aug 11, 2018 at 09:54:01AM -0400, Aaron Conole wrote: > >> > Ben Pfaff writes: > >> > > >> > > This also makes a start at a testsuite for checkpatch. > >> > > > >> > > CC: Aaron Conole > >> > > Signed-off-by: Ben Pfaff > >> > > --- > >> > > >> > Acked-by: Aaron Conole > >> > > >> > Thanks for this work, Ben! > >> > > >> > There's one more case that we could possibly consider (but it's rare > >> > enough that maybe it's not that big of a deal to let it stand for now), > >> > encompassed by commit 52e20a3d6c8b. > >> > > >> > Once this goes in I'll fix the bot up to add a signoff (so that the > >> > warning is squelched :). > >> > >> I guess another way would be to make checkpatch ignore some particular > >> committer, e.g. "Nobody " and not complain about the > >> lack of sign-off. > > > > Or the robot could just use the author as committer as well. > > I'll use that option. OK. I applied this to master. Thank you for all the reviews! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] checkpatch: Improve accuracy and specificity of sign-off checking.
Ben Pfaff writes: > On Sun, Aug 12, 2018 at 07:43:14AM -0700, Ben Pfaff wrote: >> On Sat, Aug 11, 2018 at 09:54:01AM -0400, Aaron Conole wrote: >> > Ben Pfaff writes: >> > >> > > This also makes a start at a testsuite for checkpatch. >> > > >> > > CC: Aaron Conole >> > > Signed-off-by: Ben Pfaff >> > > --- >> > >> > Acked-by: Aaron Conole >> > >> > Thanks for this work, Ben! >> > >> > There's one more case that we could possibly consider (but it's rare >> > enough that maybe it's not that big of a deal to let it stand for now), >> > encompassed by commit 52e20a3d6c8b. >> > >> > Once this goes in I'll fix the bot up to add a signoff (so that the >> > warning is squelched :). >> >> I guess another way would be to make checkpatch ignore some particular >> committer, e.g. "Nobody " and not complain about the >> lack of sign-off. > > Or the robot could just use the author as committer as well. I'll use that option. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] checkpatch: Improve accuracy and specificity of sign-off checking.
On Sun, Aug 12, 2018 at 07:43:14AM -0700, Ben Pfaff wrote: > On Sat, Aug 11, 2018 at 09:54:01AM -0400, Aaron Conole wrote: > > Ben Pfaff writes: > > > > > This also makes a start at a testsuite for checkpatch. > > > > > > CC: Aaron Conole > > > Signed-off-by: Ben Pfaff > > > --- > > > > Acked-by: Aaron Conole > > > > Thanks for this work, Ben! > > > > There's one more case that we could possibly consider (but it's rare > > enough that maybe it's not that big of a deal to let it stand for now), > > encompassed by commit 52e20a3d6c8b. > > > > Once this goes in I'll fix the bot up to add a signoff (so that the > > warning is squelched :). > > I guess another way would be to make checkpatch ignore some particular > committer, e.g. "Nobody " and not complain about the > lack of sign-off. Or the robot could just use the author as committer as well. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] checkpatch: Improve accuracy and specificity of sign-off checking.
On Sat, Aug 11, 2018 at 09:54:01AM -0400, Aaron Conole wrote: > Ben Pfaff writes: > > > This also makes a start at a testsuite for checkpatch. > > > > CC: Aaron Conole > > Signed-off-by: Ben Pfaff > > --- > > Acked-by: Aaron Conole > > Thanks for this work, Ben! > > There's one more case that we could possibly consider (but it's rare > enough that maybe it's not that big of a deal to let it stand for now), > encompassed by commit 52e20a3d6c8b. > > Once this goes in I'll fix the bot up to add a signoff (so that the > warning is squelched :). I guess another way would be to make checkpatch ignore some particular committer, e.g. "Nobody " and not complain about the lack of sign-off. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] checkpatch: Improve accuracy and specificity of sign-off checking.
Ben Pfaff writes: > This also makes a start at a testsuite for checkpatch. > > CC: Aaron Conole > Signed-off-by: Ben Pfaff > --- Acked-by: Aaron Conole Thanks for this work, Ben! There's one more case that we could possibly consider (but it's rare enough that maybe it's not that big of a deal to let it stand for now), encompassed by commit 52e20a3d6c8b. Once this goes in I'll fix the bot up to add a signoff (so that the warning is squelched :). ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3] checkpatch: Improve accuracy and specificity of sign-off checking.
This also makes a start at a testsuite for checkpatch.
CC: Aaron Conole
Signed-off-by: Ben Pfaff
---
v1->v2: Fix case where there's no committer and add test for it.
v2->v3: Fix my misconceptions about "git log" patch formatting.
tests/automake.mk | 1 +
tests/checkpatch.at | 158
tests/testsuite.at | 1 +
utilities/checkpatch.py | 70 ++---
4 files changed, 220 insertions(+), 10 deletions(-)
create mode 100644 tests/checkpatch.at
diff --git a/tests/automake.mk b/tests/automake.mk
index 8224e5a4a22d..01f5077cd6ef 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -24,6 +24,7 @@ COMMON_MACROS_AT = \
TESTSUITE_AT = \
tests/testsuite.at \
tests/completion.at \
+ tests/checkpatch.at \
tests/library.at \
tests/heap.at \
tests/bundle.at \
diff --git a/tests/checkpatch.at b/tests/checkpatch.at
new file mode 100644
index ..bcfb753784ca
--- /dev/null
+++ b/tests/checkpatch.at
@@ -0,0 +1,158 @@
+AT_BANNER([checkpatch])
+
+OVS_START_SHELL_HELPERS
+# try_checkpatch PATCH [ERRORS]
+#
+# Runs checkpatch under Python 2 and Python 3, if installed, on the given
+# PATCH, expecting the specified set of ERRORS (and warnings).
+try_checkpatch() {
+AT_SKIP_IF([test $HAVE_PYTHON2 = no && test $HAVE_PYTHON3 = no])
+# 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.
+echo "$1" | sed 's/^//' > test.patch
+if grep '---' expout >/dev/null 2>&1; then :
+else
+printf '\n---\n' >> test.patch
+fi
+
+# Take expected output from $2.
+if test -n "$2"; then
+echo "$2" | sed 's/^//' > expout
+else
+: > expout
+fi
+
+try_checkpatch__ "$HAVE_PYTHON2" "$PYTHON2"
+try_checkpatch__ "$HAVE_PYTHON3" "$PYTHON3"
+}
+try_checkpatch__() {
+if test $1 = no; then
+:
+elif test -s expout; then
+AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch],
+ [255], [stdout])
+AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
+else
+AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch])
+fi
+}
+OVS_END_SHELL_HELPERS
+
+AT_SETUP([checkpatch - sign-offs])
+
+# Sign-off for single author who is also the committer.
+try_checkpatch \
+ "Author: A
+Commit: A
+
+Signed-off-by: A"
+try_checkpatch \
+ "Author: A
+Commit: A" \
+ "ERROR: Author A needs to sign off."
+
+# Sign-off for single author and different committer.
+try_checkpatch \
+ "Author: A
+Commit: B
+
+Signed-off-by: A
+Signed-off-by: B"
+try_checkpatch \
+ "Author: A
+Commit: B" \
+ "ERROR: Author A needs to sign off.
+ERROR: Committer B needs to sign off."
+
+# Sign-off for multiple authors with one author also the committer.
+try_checkpatch \
+ "Author: A
+Commit: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B"
+try_checkpatch \
+ "Author: A
+Commit: A
+
+Co-authored-by: B
+Signed-off-by: B" \
+ "ERROR: Author A needs to sign off."
+try_checkpatch \
+ "Author: A
+Commit: A
+
+Signed-off-by: A
+Co-authored-by: B" \
+ "ERROR: Co-author B needs to sign off."
+try_checkpatch \
+ "Author: A
+Commit: A
+
+Co-authored-by: B" \
+ "ERROR: Author A needs to sign off.
+ERROR: Co-author B needs to sign off."
+
+# Sign-off for multiple authors and separate committer.
+try_checkpatch \
+ "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C"
+try_checkpatch \
+ "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B" \
+ "ERROR: Committer C needs to sign off."
+
+# Extra sign-offs:
+#
+#- If we know the committer, one extra sign-off raises a warning.
+#
+#- If we do not know the committer, two extra sign-offs raise a warning.
+try_checkpatch \
+ "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C
+Signed-off-by: D" \
+ "WARNING: Unexpected sign-offs from developers who are not authors or
co-authors or committers: D"
+try_checkpatch \
+ "Author: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C"
+try_checkpatch \
+ "Author: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C
+Signed-off-by: D" \
+ "WARNING: Unexpected sign-offs from developers who are not authors or
co-authors or committers: C, D"
+
+# Missing committer is OK, missing author is an error.
+try_checkpatch \
+ "Author: A
+
+Signed-off-by: A"
+try_checkpatch \
+ "Commit: A
+
+Signed-off-by: A" \
+ "ERROR: Patch lacks author."
+
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 15c385e2cddb..690904e30881 10064
