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