diff mbox series

[ovs-dev] checkpatch: Add new check-authors-file option to checkpatch.py.

Message ID ee628471204a23a3cf51e4be7327bcae2e06673d.1727095591.git.echaudro@redhat.com
State Superseded
Delegated to: Eelco Chaudron
Headers show
Series [ovs-dev] checkpatch: Add new check-authors-file option to checkpatch.py. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Eelco Chaudron Sept. 23, 2024, 12:46 p.m. UTC
This patch adds a new option, --check-authors-file, to the checkpatch
tool to help OVS maintainers check for missing authors in the
AUTHORS.rst file.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 utilities/checkpatch.py | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Comments

Eelco Chaudron Sept. 24, 2024, 7:55 a.m. UTC | #1
On 23 Sep 2024, at 14:46, Eelco Chaudron wrote:

> This patch adds a new option, --check-authors-file, to the checkpatch
> tool to help OVS maintainers check for missing authors in the
> AUTHORS.rst file.
>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>

Recheck-request: github-robot

> ---
>  utilities/checkpatch.py | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index 742a0bc47..fe18b1674 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -28,6 +28,7 @@ __errors = 0
>  __warnings = 0
>  empty_return_check_state = 0
>  print_file_name = None
> +check_authors_file = False
>  checking_file = False
>  total_line = 0
>  colors = False
> @@ -977,6 +978,19 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None):
>                                        "who are not authors or co-authors or "
>                                        "committers: %s"
>                                        % ", ".join(extra_sigs))
> +
> +                if check_authors_file or author:
> +                    m = re.search(r'<(.*?)>', author)
> +                    if m:
> +                        try:
> +                            with open("AUTHORS.rst", "r") as file:
> +                                if m.group(1) not in file.read():
> +                                    print_error("Author '%s' is not in the "
> +                                    "AUTHORS.rst file!" % author)
> +                        except FileNotFoundError:
> +                            print_error("Can't open AUTHORS.rst file in "
> +                                        "current path!")
> +
>              elif is_committer.match(line):
>                  committer = is_committer.match(line).group(2)
>              elif is_author.match(line):
> @@ -1067,6 +1081,7 @@ Input options:
>
>  Check options:
>  -h|--help                      This help message
> +-a|--check-authors-file        Check AUTHORS file for existense of author
>  -b|--skip-block-whitespace     Skips the if/while/for whitespace tests
>  -l|--skip-leading-whitespace   Skips the leading whitespace test
>  -q|--quiet                     Only print error and warning information
> @@ -1138,7 +1153,7 @@ if __name__ == '__main__':
>                                            sys.argv[1:])
>          n_patches = int(numeric_options[-1][1:]) if numeric_options else 0
>
> -        optlist, args = getopt.getopt(args, 'bhlstfSq',
> +        optlist, args = getopt.getopt(args, 'abhlstfSq',
>                                        ["check-file",
>                                         "help",
>                                         "skip-block-whitespace",
> @@ -1157,6 +1172,8 @@ if __name__ == '__main__':
>          if o in ("-h", "--help"):
>              usage()
>              sys.exit(0)
> +        elif o in ("-a", "--check-authors-file"):
> +            check_authors_file = True
>          elif o in ("-b", "--skip-block-whitespace"):
>              skip_block_whitespace_check = True
>          elif o in ("-l", "--skip-leading-whitespace"):
> -- 
> 2.46.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Simon Horman Sept. 24, 2024, 2:43 p.m. UTC | #2
On Mon, Sep 23, 2024 at 02:46:31PM +0200, Eelco Chaudron wrote:
> This patch adds a new option, --check-authors-file, to the checkpatch
> tool to help OVS maintainers check for missing authors in the
> AUTHORS.rst file.

Nice. I have missed checking this too many times.

> 
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
>  utilities/checkpatch.py | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index 742a0bc47..fe18b1674 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -28,6 +28,7 @@ __errors = 0
>  __warnings = 0
>  empty_return_check_state = 0
>  print_file_name = None
> +check_authors_file = False
>  checking_file = False
>  total_line = 0
>  colors = False
> @@ -977,6 +978,19 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None):
>                                        "who are not authors or co-authors or "
>                                        "committers: %s"
>                                        % ", ".join(extra_sigs))
> +
> +                if check_authors_file or author:
> +                    m = re.search(r'<(.*?)>', author)
> +                    if m:
> +                        try:
> +                            with open("AUTHORS.rst", "r") as file:
> +                                if m.group(1) not in file.read():
> +                                    print_error("Author '%s' is not in the "
> +                                    "AUTHORS.rst file!" % author)

I suppose a false positive is unlikely, but this will pass
on partial matches of email addresses.

F.e. orms@ovn.or will match the entry for myself

> +                        except FileNotFoundError:
> +                            print_error("Can't open AUTHORS.rst file in "
> +                                        "current path!")
> +
>              elif is_committer.match(line):
>                  committer = is_committer.match(line).group(2)
>              elif is_author.match(line):
> @@ -1067,6 +1081,7 @@ Input options:
>  
>  Check options:
>  -h|--help                      This help message
> +-a|--check-authors-file        Check AUTHORS file for existense of author
>  -b|--skip-block-whitespace     Skips the if/while/for whitespace tests
>  -l|--skip-leading-whitespace   Skips the leading whitespace test
>  -q|--quiet                     Only print error and warning information
> @@ -1138,7 +1153,7 @@ if __name__ == '__main__':
>                                            sys.argv[1:])
>          n_patches = int(numeric_options[-1][1:]) if numeric_options else 0
>  
> -        optlist, args = getopt.getopt(args, 'bhlstfSq',
> +        optlist, args = getopt.getopt(args, 'abhlstfSq',
>                                        ["check-file",
>                                         "help",
>                                         "skip-block-whitespace",

I think you need this too:

@@ -1156,6 +1156,7 @@ if __name__ == '__main__':
         optlist, args = getopt.getopt(args, 'abhlstfSq',
                                       ["check-file",
                                        "help",
+                                       "check-authors-file",
                                        "skip-block-whitespace",
                                        "skip-leading-whitespace",
                                        "skip-signoff-lines",

> @@ -1157,6 +1172,8 @@ if __name__ == '__main__':
>          if o in ("-h", "--help"):
>              usage()
>              sys.exit(0)
> +        elif o in ("-a", "--check-authors-file"):
> +            check_authors_file = True
>          elif o in ("-b", "--skip-block-whitespace"):
>              skip_block_whitespace_check = True
>          elif o in ("-l", "--skip-leading-whitespace"):
> -- 
> 2.46.0
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Eelco Chaudron Sept. 25, 2024, 2:09 p.m. UTC | #3
On 24 Sep 2024, at 16:43, Simon Horman wrote:

> On Mon, Sep 23, 2024 at 02:46:31PM +0200, Eelco Chaudron wrote:
>> This patch adds a new option, --check-authors-file, to the checkpatch
>> tool to help OVS maintainers check for missing authors in the
>> AUTHORS.rst file.
>
> Nice. I have missed checking this too many times.

Thanks for the review Simon! I’ve made the changes you suggested below and sent out a v2.

Cheers,

Eelco

>>
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>> ---
>>  utilities/checkpatch.py | 19 ++++++++++++++++++-
>>  1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
>> index 742a0bc47..fe18b1674 100755
>> --- a/utilities/checkpatch.py
>> +++ b/utilities/checkpatch.py
>> @@ -28,6 +28,7 @@ __errors = 0
>>  __warnings = 0
>>  empty_return_check_state = 0
>>  print_file_name = None
>> +check_authors_file = False
>>  checking_file = False
>>  total_line = 0
>>  colors = False
>> @@ -977,6 +978,19 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None):
>>                                        "who are not authors or co-authors or "
>>                                        "committers: %s"
>>                                        % ", ".join(extra_sigs))
>> +
>> +                if check_authors_file or author:
>> +                    m = re.search(r'<(.*?)>', author)
>> +                    if m:
>> +                        try:
>> +                            with open("AUTHORS.rst", "r") as file:
>> +                                if m.group(1) not in file.read():
>> +                                    print_error("Author '%s' is not in the "
>> +                                    "AUTHORS.rst file!" % author)
>
> I suppose a false positive is unlikely, but this will pass
> on partial matches of email addresses.
>
> F.e. orms@ovn.or will match the entry for myself
>
>> +                        except FileNotFoundError:
>> +                            print_error("Can't open AUTHORS.rst file in "
>> +                                        "current path!")
>> +
>>              elif is_committer.match(line):
>>                  committer = is_committer.match(line).group(2)
>>              elif is_author.match(line):
>> @@ -1067,6 +1081,7 @@ Input options:
>>
>>  Check options:
>>  -h|--help                      This help message
>> +-a|--check-authors-file        Check AUTHORS file for existense of author
>>  -b|--skip-block-whitespace     Skips the if/while/for whitespace tests
>>  -l|--skip-leading-whitespace   Skips the leading whitespace test
>>  -q|--quiet                     Only print error and warning information
>> @@ -1138,7 +1153,7 @@ if __name__ == '__main__':
>>                                            sys.argv[1:])
>>          n_patches = int(numeric_options[-1][1:]) if numeric_options else 0
>>
>> -        optlist, args = getopt.getopt(args, 'bhlstfSq',
>> +        optlist, args = getopt.getopt(args, 'abhlstfSq',
>>                                        ["check-file",
>>                                         "help",
>>                                         "skip-block-whitespace",
>
> I think you need this too:
>
> @@ -1156,6 +1156,7 @@ if __name__ == '__main__':
>          optlist, args = getopt.getopt(args, 'abhlstfSq',
>                                        ["check-file",
>                                         "help",
> +                                       "check-authors-file",
>                                         "skip-block-whitespace",
>                                         "skip-leading-whitespace",
>                                         "skip-signoff-lines",
>
>> @@ -1157,6 +1172,8 @@ if __name__ == '__main__':
>>          if o in ("-h", "--help"):
>>              usage()
>>              sys.exit(0)
>> +        elif o in ("-a", "--check-authors-file"):
>> +            check_authors_file = True
>>          elif o in ("-b", "--skip-block-whitespace"):
>>              skip_block_whitespace_check = True
>>          elif o in ("-l", "--skip-leading-whitespace"):
>> -- 
>> 2.46.0
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
diff mbox series

Patch

diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 742a0bc47..fe18b1674 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -28,6 +28,7 @@  __errors = 0
 __warnings = 0
 empty_return_check_state = 0
 print_file_name = None
+check_authors_file = False
 checking_file = False
 total_line = 0
 colors = False
@@ -977,6 +978,19 @@  def ovs_checkpatch_parse(text, filename, author=None, committer=None):
                                       "who are not authors or co-authors or "
                                       "committers: %s"
                                       % ", ".join(extra_sigs))
+
+                if check_authors_file or author:
+                    m = re.search(r'<(.*?)>', author)
+                    if m:
+                        try:
+                            with open("AUTHORS.rst", "r") as file:
+                                if m.group(1) not in file.read():
+                                    print_error("Author '%s' is not in the "
+                                    "AUTHORS.rst file!" % author)
+                        except FileNotFoundError:
+                            print_error("Can't open AUTHORS.rst file in "
+                                        "current path!")
+
             elif is_committer.match(line):
                 committer = is_committer.match(line).group(2)
             elif is_author.match(line):
@@ -1067,6 +1081,7 @@  Input options:
 
 Check options:
 -h|--help                      This help message
+-a|--check-authors-file        Check AUTHORS file for existense of author
 -b|--skip-block-whitespace     Skips the if/while/for whitespace tests
 -l|--skip-leading-whitespace   Skips the leading whitespace test
 -q|--quiet                     Only print error and warning information
@@ -1138,7 +1153,7 @@  if __name__ == '__main__':
                                           sys.argv[1:])
         n_patches = int(numeric_options[-1][1:]) if numeric_options else 0
 
-        optlist, args = getopt.getopt(args, 'bhlstfSq',
+        optlist, args = getopt.getopt(args, 'abhlstfSq',
                                       ["check-file",
                                        "help",
                                        "skip-block-whitespace",
@@ -1157,6 +1172,8 @@  if __name__ == '__main__':
         if o in ("-h", "--help"):
             usage()
             sys.exit(0)
+        elif o in ("-a", "--check-authors-file"):
+            check_authors_file = True
         elif o in ("-b", "--skip-block-whitespace"):
             skip_block_whitespace_check = True
         elif o in ("-l", "--skip-leading-whitespace"):