Message ID | 1460051355-32289-1-git-send-email-joe@ovn.org |
---|---|
State | Accepted |
Headers | show |
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.
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)