| Message ID | 2969aeb38433ca58c02b5efdc9000a8000e66680.1631264711.git.tredaelli@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] checkpatch: check if some tags are wrongly written | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Fri, Sep 10, 2021 at 11:08 AM Timothy Redaelli <tredaelli@redhat.com> wrote: > > Currently, there are some patches with the tags wrongly written (with > space instead of dash ) and this may prevent some automatic system or CI > to detect them correctly. > > This commit adds a check in checkpatch to be sure the tag is written > correctly with dash and not with space. > > The tags supported by the commit are: > Acked-by, Reported-at, Reported-by, Requested-by, Reviewed-by, Submitted-at > and Suggested-by. > > It's not necessary to add "Signed-off-by" since it's already checked in > checkpatch. Just want to be sure I understand. checkpatch reports missing sob, but won't flag incorrectly written tags. For example: Author: A Signed-off-by: A Signed off by: B I don't think this would be detected. Otherwise, the patch lgtm, the test runs fine. Thanks.
On 10/4/21 16:03, David Marchand wrote: > On Fri, Sep 10, 2021 at 11:08 AM Timothy Redaelli <tredaelli@redhat.com> wrote: >> >> Currently, there are some patches with the tags wrongly written (with >> space instead of dash ) and this may prevent some automatic system or CI >> to detect them correctly. >> >> This commit adds a check in checkpatch to be sure the tag is written >> correctly with dash and not with space. >> >> The tags supported by the commit are: >> Acked-by, Reported-at, Reported-by, Requested-by, Reviewed-by, Submitted-at >> and Suggested-by. >> >> It's not necessary to add "Signed-off-by" since it's already checked in >> checkpatch. > > Just want to be sure I understand. > checkpatch reports missing sob, but won't flag incorrectly written tags. > > For example: > Author: A > > Signed-off-by: A > Signed off by: B > > I don't think this would be detected. > > > Otherwise, the patch lgtm, the test runs fine. > Thanks. I applied this patch as is for now. Some enhancements could be added later, if needed. Thanks! Bet regards, Ilya Maximets.
diff --git a/tests/checkpatch.at b/tests/checkpatch.at index 0718acd99..246170a26 100755 --- a/tests/checkpatch.at +++ b/tests/checkpatch.at @@ -348,3 +348,63 @@ try_checkpatch \ " AT_CLEANUP + +AT_SETUP([checkpatch - malformed tags]) +try_checkpatch \ + " Author: A + + Acked by: foo... + Signed-off-by: A" \ + "ERROR: Acked-by tag is malformed. + 1: Acked by: foo... +" +try_checkpatch \ + " Author: A + + Reported at: foo... + Signed-off-by: A" \ + "ERROR: Reported-at tag is malformed. + 1: Reported at: foo... +" +try_checkpatch \ + " Author: A + + Reported by: foo... + Signed-off-by: A" \ + "ERROR: Reported-by tag is malformed. + 1: Reported by: foo... +" +try_checkpatch \ + " Author: A + + Requested by: foo... + Signed-off-by: A" \ + "ERROR: Requested-by tag is malformed. + 1: Requested by: foo... +" +try_checkpatch \ + " Author: A + + Reviewed by: foo... + Signed-off-by: A" \ + "ERROR: Reviewed-by tag is malformed. + 1: Reviewed by: foo... +" +try_checkpatch \ + " Author: A + + Submitted at: foo... + Signed-off-by: A" \ + "ERROR: Submitted-at tag is malformed. + 1: Submitted at: foo... +" +try_checkpatch \ + " Author: A + + Suggested by: foo... + Signed-off-by: A" \ + "ERROR: Suggested-by tag is malformed. + 1: Suggested by: foo... +" + +AT_CLEANUP diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index 699fb4b02..d9d90847f 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -749,6 +749,16 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): is_gerrit_change_id = re.compile(r'(\s*(change-id: )(.*))$', re.I | re.M | re.S) + tags_typos = { + r'^Acked by:': 'Acked-by:', + r'^Reported at:': 'Reported-at:', + r'^Reported by:': 'Reported-by:', + r'^Requested by:': 'Requested-by:', + r'^Reviewed by:': 'Reviewed-by:', + r'^Submitted at:': 'Submitted-at:', + r'^Suggested by:': 'Suggested-by:', + } + reset_counters() for line in text.splitlines(): @@ -838,6 +848,11 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): print("%d: %s\n" % (lineno, line)) elif spellcheck: check_spelling(line, False) + for typo, correct in tags_typos.items(): + m = re.match(typo, line, re.I) + if m: + print_error("%s tag is malformed." % (correct[:-1])) + print("%d: %s\n" % (lineno, line)) elif parse == PARSE_STATE_CHANGE_BODY: newfile = hunks.match(line)
Currently, there are some patches with the tags wrongly written (with space instead of dash ) and this may prevent some automatic system or CI to detect them correctly. This commit adds a check in checkpatch to be sure the tag is written correctly with dash and not with space. The tags supported by the commit are: Acked-by, Reported-at, Reported-by, Requested-by, Reviewed-by, Submitted-at and Suggested-by. It's not necessary to add "Signed-off-by" since it's already checked in checkpatch. Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> --- v2: - Removed multiple spaces to make flake8 happy, as reported by Aaron Conole) - Added Acked-by and Reviewed-by, as requested by Paolo Valerio. --- tests/checkpatch.at | 60 +++++++++++++++++++++++++++++++++++++++++ utilities/checkpatch.py | 15 +++++++++++ 2 files changed, 75 insertions(+)