Russell Bryant <russ...@ovn.org> writes:

> On Tue, Apr 5, 2016 at 2:17 PM, Joe Stringer <j...@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 <j...@ovn.org>
>>
>
> Acked-by: Russell Bryant <russ...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to