On 17 May 2022, at 9:20, Peng He wrote:

> You mean I can ignore the following warnings?
>
> "
> checkpatch:
> ERROR: Author Peng He <xnhp0...@gmail.com> needs to sign off.
> WARNING: Unexpected sign-offs from developers who are not authors or
> co-authors or committers: Peng He <hepeng.0...@bytedance.com>
> Lines checked: 98, Warnings: 1, Errors: 1
> "

I think these are because of your mixed email address. Ilya can probably answer 
this, but I think he will fix this on commit, Ilya?

>
> Eelco Chaudron <echau...@redhat.com> 于2022年5月17日周二 14:26写道:
>
>>
>>
>> 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
>>
>>
>
> -- 
> hepeng

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

Reply via email to