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