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

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@ovn.org>
Acked-by: Russell Bryant <russell@ovn.org>
---
v2: Create a wider blacklist of formats to check for in the filenames.
v3: Add '.m4' to the blacklist
    Improve python style
    Fix issues where patch line length checking would be entirely
    disabled after the first blacklisted file is hit.
---
 utilities/checkpatch.py | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Joe Stringer <joe@ovn.org> writes:

> 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@ovn.org>
> Acked-by: Russell Bryant <russell@ovn.org>
> ---
> v2: Create a wider blacklist of formats to check for in the filenames.
> v3: Add '.m4' to the blacklist
>     Improve python style
>     Fix issues where patch line length checking would be entirely
>     disabled after the first blacklisted file is hit.
> ---
>  utilities/checkpatch.py | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index d9011f370816..dbdcbc805b4d 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -56,6 +56,13 @@ skip_trailing_whitespace_check = False
>  skip_block_whitespace_check = False
>  skip_signoff_check = False
>  
> +# Don't enforce character limit on files that include these characters in their
> +# name, as they may have legitimate reasons to have longer lines.
> +#
> +# Python isn't checked as flake8 performs these checks during build.
> +line_length_blacklist = ['.am', '.at', 'etc', '.in', '.m4', '.mk', '.patch',
> +                         '.py']
> +
>  
>  def is_added_line(line):
>      """Returns TRUE if the line in question is an added line.
> @@ -99,14 +106,23 @@ def ovs_checkpatch_parse(text):
>      co_authors = []
>      parse = 0
>      current_file = ''
> +    previous_file = ''
>      scissors = re.compile(r'^[\w]*---[\w]*')
>      hunks = re.compile('^(---|\+\+\+) (\S+)')
>      is_signature = re.compile(r'((\s*Signed-off-by: )(.*))$',
>                                re.I | re.M | re.S)
>      is_co_author = re.compile(r'(\s*(Co-authored-by: )(.*))$',
>                                re.I | re.M | re.S)
> +    skip_line_length_check = False
>  
>      for line in text.split('\n'):
> +        if current_file != previous_file:
> +            previous_file = current_file
> +            if any([fmt in current_file for fmt in line_length_blacklist]):
> +                skip_line_length_check = True
> +            else:
> +                skip_line_length_check = False
> +
>          lineno = lineno + 1
>          if len(line) <= 0:
>              continue
> @@ -154,7 +170,7 @@ 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:
> +            if len(line[1:]) > 79 and not skip_line_length_check:
>                  print_line = True
>                  print_warning("Line is greater than 79-characters long",
>                                lineno)

LGTM,

Tested-by: Aaron Conole <aconole@redhat.com>

Thanks for this!
-Aaron

On 7 April 2016 at 10:59, Aaron Conole <aconole@redhat.com> wrote:
> Joe Stringer <joe@ovn.org> writes:
>
>> 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@ovn.org>
>> Acked-by: Russell Bryant <russell@ovn.org>
>> ---
>> v2: Create a wider blacklist of formats to check for in the filenames.
>> v3: Add '.m4' to the blacklist
>>     Improve python style
>>     Fix issues where patch line length checking would be entirely
>>     disabled after the first blacklisted file is hit.
>> ---
>>  utilities/checkpatch.py | 18 +++++++++++++++++-
>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
>> index d9011f370816..dbdcbc805b4d 100755
>> --- a/utilities/checkpatch.py
>> +++ b/utilities/checkpatch.py
>> @@ -56,6 +56,13 @@ skip_trailing_whitespace_check = False
>>  skip_block_whitespace_check = False
>>  skip_signoff_check = False
>>
>> +# Don't enforce character limit on files that include these characters in their
>> +# name, as they may have legitimate reasons to have longer lines.
>> +#
>> +# Python isn't checked as flake8 performs these checks during build.
>> +line_length_blacklist = ['.am', '.at', 'etc', '.in', '.m4', '.mk', '.patch',
>> +                         '.py']
>> +
>>
>>  def is_added_line(line):
>>      """Returns TRUE if the line in question is an added line.
>> @@ -99,14 +106,23 @@ def ovs_checkpatch_parse(text):
>>      co_authors = []
>>      parse = 0
>>      current_file = ''
>> +    previous_file = ''
>>      scissors = re.compile(r'^[\w]*---[\w]*')
>>      hunks = re.compile('^(---|\+\+\+) (\S+)')
>>      is_signature = re.compile(r'((\s*Signed-off-by: )(.*))$',
>>                                re.I | re.M | re.S)
>>      is_co_author = re.compile(r'(\s*(Co-authored-by: )(.*))$',
>>                                re.I | re.M | re.S)
>> +    skip_line_length_check = False
>>
>>      for line in text.split('\n'):
>> +        if current_file != previous_file:
>> +            previous_file = current_file
>> +            if any([fmt in current_file for fmt in line_length_blacklist]):
>> +                skip_line_length_check = True
>> +            else:
>> +                skip_line_length_check = False
>> +
>>          lineno = lineno + 1
>>          if len(line) <= 0:
>>              continue
>> @@ -154,7 +170,7 @@ 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:
>> +            if len(line[1:]) > 79 and not skip_line_length_check:
>>                  print_line = True
>>                  print_warning("Line is greater than 79-characters long",
>>                                lineno)
>
> LGTM,
>
> Tested-by: Aaron Conole <aconole@redhat.com>
>
> Thanks for this!
> -Aaron

Thanks, I pushed this to master.