[ovs-dev] [PATCH] checkpatch: Don't enforce char limit on tests.

Joe Stringer joe at ovn.org
Wed Apr 6 19:44:32 UTC 2016


On 6 April 2016 at 07:33, Aaron Conole <aconole at redhat.com> wrote:
> Russell Bryant <russell at ovn.org> writes:
>
>> On Tue, Apr 5, 2016 at 2:17 PM, Joe Stringer <joe at ovn.org> wrote:
>>
>>> Although tests ideally also stick to shorter line lengths, it is very
>>> common for fixed text blocks like flows or large packets to be specified
>>> within tests. Checkpatch shouldn't complain about cases like these.
>>>
>>> Signed-off-by: Joe Stringer <joe at ovn.org>
>>>
>>
>> Acked-by: Russell Bryant <russell at ovn.org>
>>
>> Some thoughts below ...
>>
>> ---
>>>  utilities/checkpatch.py | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
>>> index d9011f370816..791c7d902fa5 100755
>>> --- a/utilities/checkpatch.py
>>> +++ b/utilities/checkpatch.py
>>> @@ -154,7 +154,10 @@ def ovs_checkpatch_parse(text):
>>>              if trailing_whitespace_or_crlf(line[1:]):
>>>                  print_line = True
>>>                  print_warning("Line has trailing whitespace", lineno)
>>> -            if len(line[1:]) > 79:
>>> +
>>> +            # Don't enforce character limit on test files, since they
>>> commonly
>>> +            # include long pieces of text like flows or hex dumps of
>>> packets
>>> +            if len(line[1:]) > 79 and '.at' not in current_file:
>>>                  print_line = True
>>>                  print_warning("Line is greater than 79-characters long",
>>>                                lineno)
>>
>>
>> I believe there are other examples that would hit this same problem, such
>> as *.am at least.  flake8 is already checking the equivalent for Python
>> files.  An alternative would be to just whitelist what we want to check.
>> (.c, .h, .md?)
>
> I think either a whitelist or blacklist work fine for this check. There are
> certain files which just don't make sense to check for line
> lengths.
>
> That said, the proposed patch looks good to me.

Thanks for the reviews, I'll send a follow up (in case my python style
is a bit too C-like ;) )



More information about the dev mailing list