The only issue I have is that I have to use the company's email address to
submit patches
while I am using the personal email to subscribe to the maillist.

So the bot will always warn about the author needing to sign-off.
Perhaps in the future I would use both email addresses to avoid this
warning.


Eelco Chaudron <echau...@redhat.com> 于2022年5月16日周一 19:36写道:

>
>
> On 16 May 2022, at 12:19, Peng He wrote:
>
> > Eelco Chaudron <echau...@redhat.com> 于2022年5月16日周一 17:12写道:
> >
> >>
> >>
> >> On 14 May 2022, at 10:40, Peng He wrote:
> >>
> >>> Signed-off-by: Peng He <hepeng.0...@bytedance.com>
> >>> ---
> >>>  utilities/checkpatch.py | 32 +++++++++++++++++++++++++++++---
> >>>  1 file changed, 29 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> >>> index 8c7faa419..67c517b69 100755
> >>> --- a/utilities/checkpatch.py
> >>> +++ b/utilities/checkpatch.py
> >>> @@ -619,6 +619,8 @@ def regex_function_factory(func_name):
> >>>  def regex_error_factory(description):
> >>>      return lambda: print_error(description)
> >>>
> >>
> >> Need extra newline, install flake8 and you will get the error at compile
> >> time.
> >>
> >>> +def regex_warn_factory(description):
> >>> +    return lambda: print_warning(description)
> >>
> >> Need extra newline, install flake8 and you will get the error at compile
> >> time.
> >>
> >>>
> >>>  std_functions = [
> >>>          ('malloc', 'Use xmalloc() in place of malloc()'),
> >>> @@ -636,6 +638,7 @@ std_functions = [
> >>>          ('assert', 'Use ovs_assert() in place of assert()'),
> >>>          ('error', 'Use ovs_error() in place of error()'),
> >>>  ]
> >>> +
> >>>  checks += [
> >>>      {'regex': r'(\.c|\.h)(\.in)?$',
> >>>       'match_name': None,
> >>> @@ -644,6 +647,21 @@ checks += [
> >>>       'print': regex_error_factory(description)}
> >>>      for (function_name, description) in std_functions]
> >>>
> >>> +experimental_api = [
> >>
> >> I do not think this is an experimental API, I would call it something
> like
> >> a suspicious API maybe?
> >>
> >
> > or change to easy_to_misused_api?
>
> This is fine by me to.
>
> >>
> >>> +        ('ovsrcu_barrier',
> >>> +            'lib/ovs-rcu.c',
> >>> +            'Are you sure you need to use ovsrcu_barrier(),'
> >>
> >> Here you need to add a space at the end, as the error message now looks
> >> like:
> >>
> >>   WARNING: Are you sure you need to use ovsrcu_barrier(),in most cases
> >> ovsrcu_synchronize() will be fine?
> >>
> >>> +            'in most cases ovsrcu_synchronize() will be fine?'),
> >>> +        ]
> >>> +
> >>> +checks += [
> >>> +    {'regex': r'(\.c)(\.in)?$',
> >>> +     'match_name': lambda x: x != location,
> >>> +     'prereq': lambda x: not is_comment_line(x),
> >>> +     'check': regex_function_factory(function_name),
> >>> +     'print': regex_warn_factory(description)}
> >>> +    for (function_name, location, description) in experimental_api]
> >>> +
> >>>
> >>>  def regex_operator_factory(operator):
> >>>      regex = re.compile(r'^[^#][^"\']*[^ "]%s[^ "\'][^"]*' % operator)
> >>> @@ -676,12 +694,20 @@ def get_file_type_checks(filename):
> >>>      global checks
> >>>      checkList = []
> >>>      for check in checks:
> >>> +        regex_check = True
> >>> +        match_check = True
> >>> +
> >>>          if check['regex'] is None and check['match_name'] is None:
> >>>              checkList.append(check)
> >>> +            continue
> >>> +
> >>>          if check['regex'] is not None and \
> >>> -           re.compile(check['regex']).search(filename) is not None:
> >>> -            checkList.append(check)
> >>> -        elif check['match_name'] is not None and
> >> check['match_name'](filename):
> >>> +           re.compile(check['regex']).search(filename) is None:
> >>> +            regex_check = False
> >>> +        elif check['match_name'] is not None and not
> >> check['match_name'](filename):
> >>
> >> Line is too long so you need to break it up:
> >>
> >> utilities/checkpatch.py:709:80: E501 line too long (83 > 79 characters)
> >>
> >>
> >>> +            match_check = False
> >>> +
> >>> +        if regex_check and match_check:
> >>>              checkList.append(check)
> >>
> >> Would it not make more sense to re-write the above elif cases to a
> single
> >> case?
> >>
> >>>      return checkList
> >>>
> >>> --
> >>> 2.25.1
> >>
> >>
> > will send a new version, thanks for the detailed review.
>
> You can also add my Signed-off-by: if you make the suggested changes to
> keep the robot happy.
>
> //Eelco
>
>

-- 
hepeng
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to