On 17 May 2022, at 7:49, Peng He wrote:

> 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.

There is this error on patch 1/3:

ERROR: Co-author Eelco Chaudron <echau...@redhat.com> needs to sign off.

So I guess you need to add my sign-off also in that patch.
>
>
> 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