On 6 April 2016 at 07:33, Aaron Conole <acon...@redhat.com> wrote:
> 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.
Thanks for the reviews, I'll send a follow up (in case my python style
is a bit too C-like ;) )
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
