| Message ID | 20200611100932.23783-1-roid@mellanox.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] checkpatch: Increase recommended line length to 100 | expand |
On 2020-06-11 1:09 PM, Roi Dayan wrote: > This is to match a recent kernel checkpatch change that > also increased it to 100 line length. > > Signed-off-by: Roi Dayan <roid@mellanox.com> > --- > utilities/checkpatch.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > index fc9e20bf1b5f..80ee6d827567 100755 > --- a/utilities/checkpatch.py > +++ b/utilities/checkpatch.py > @@ -287,8 +287,8 @@ def pointer_whitespace_check(line): > > def line_length_check(line): > """Return TRUE if the line length is too long""" > - if len(line) > 79: > - print_warning("Line is %d characters long (recommended limit is 79)" > + if len(line) > 100: > + print_warning("Line is %d characters long (recommended limit is 100)" > % len(line)) > return True > return False > Hi, Any comments on this? Thanks, Roi
On Tue, Jun 16, 2020 at 04:14:55PM +0300, Roi Dayan wrote: > > > On 2020-06-11 1:09 PM, Roi Dayan wrote: > > This is to match a recent kernel checkpatch change that > > also increased it to 100 line length. > > > > Signed-off-by: Roi Dayan <roid@mellanox.com> > > --- > > utilities/checkpatch.py | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > > index fc9e20bf1b5f..80ee6d827567 100755 > > --- a/utilities/checkpatch.py > > +++ b/utilities/checkpatch.py > > @@ -287,8 +287,8 @@ def pointer_whitespace_check(line): > > > > def line_length_check(line): > > """Return TRUE if the line length is too long""" > > - if len(line) > 79: > > - print_warning("Line is %d characters long (recommended limit is 79)" > > + if len(line) > 100: > > + print_warning("Line is %d characters long (recommended limit is 100)" > > % len(line)) > > return True > > return False > > > > > Hi, > > Any comments on this? Reviewed-by: Simon Horman <simon.horman@netronome.com>
Simon Horman <simon.horman@netronome.com> writes: > On Tue, Jun 16, 2020 at 04:14:55PM +0300, Roi Dayan wrote: >> >> >> On 2020-06-11 1:09 PM, Roi Dayan wrote: >> > This is to match a recent kernel checkpatch change that >> > also increased it to 100 line length. >> > >> > Signed-off-by: Roi Dayan <roid@mellanox.com> >> > --- >> > utilities/checkpatch.py | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py >> > index fc9e20bf1b5f..80ee6d827567 100755 >> > --- a/utilities/checkpatch.py >> > +++ b/utilities/checkpatch.py >> > @@ -287,8 +287,8 @@ def pointer_whitespace_check(line): >> > >> > def line_length_check(line): >> > """Return TRUE if the line length is too long""" >> > - if len(line) > 79: >> > - print_warning("Line is %d characters long (recommended limit is 79)" >> > + if len(line) > 100: >> > + print_warning("Line is %d characters long (recommended limit is 100)" >> > % len(line)) >> > return True >> > return False >> > >> >> >> Hi, >> >> Any comments on this? > > Reviewed-by: Simon Horman <simon.horman@netronome.com> Acked-by: Aaron Conole <aconole@redhat.com>
On 2020-06-16 5:35 PM, Aaron Conole wrote: > Simon Horman <simon.horman@netronome.com> writes: > >> On Tue, Jun 16, 2020 at 04:14:55PM +0300, Roi Dayan wrote: >>> >>> >>> On 2020-06-11 1:09 PM, Roi Dayan wrote: >>>> This is to match a recent kernel checkpatch change that >>>> also increased it to 100 line length. >>>> >>>> Signed-off-by: Roi Dayan <roid@mellanox.com> >>>> --- >>>> utilities/checkpatch.py | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py >>>> index fc9e20bf1b5f..80ee6d827567 100755 >>>> --- a/utilities/checkpatch.py >>>> +++ b/utilities/checkpatch.py >>>> @@ -287,8 +287,8 @@ def pointer_whitespace_check(line): >>>> >>>> def line_length_check(line): >>>> """Return TRUE if the line length is too long""" >>>> - if len(line) > 79: >>>> - print_warning("Line is %d characters long (recommended limit is 79)" >>>> + if len(line) > 100: >>>> + print_warning("Line is %d characters long (recommended limit is 100)" >>>> % len(line)) >>>> return True >>>> return False >>>> >>> >>> >>> Hi, >>> >>> Any comments on this? >> >> Reviewed-by: Simon Horman <simon.horman@netronome.com> > > Acked-by: Aaron Conole <aconole@redhat.com> > thanks. there are no other comments. can we merge this?
On Thu, Jun 11, 2020 at 01:09:32PM +0300, Roi Dayan wrote: > This is to match a recent kernel checkpatch change that > also increased it to 100 line length. > > Signed-off-by: Roi Dayan <roid@mellanox.com> I don't know what you effect you want from this change. If you want to keep checkpatch from complaining for long lines in kernel code, then this is not the right way, because checkpatch ignores the datapath/ directory that contains kernel code. If you want to update OVS coding style to allow longer lines in OVS's own code, then the patch should update Documentation/internals/contributing/coding-style.rst in a few different places, and we should probably discuss it more extensively than just a bare ack.
On 2020-06-18 7:51 PM, Ben Pfaff wrote: > On Thu, Jun 11, 2020 at 01:09:32PM +0300, Roi Dayan wrote: >> This is to match a recent kernel checkpatch change that >> also increased it to 100 line length. >> >> Signed-off-by: Roi Dayan <roid@mellanox.com> > > I don't know what you effect you want from this change. > > If you want to keep checkpatch from complaining for long lines in kernel > code, then this is not the right way, because checkpatch ignores the > datapath/ directory that contains kernel code. My intention was actually not for the datapath/ directory. I am using it when submitting changes to ovs. I just mentioned I want to match as the kernel because I was thinking it probably was first set to 79 to match the kernel at first. > > If you want to update OVS coding style to allow longer lines in OVS's > own code, then the patch should update > Documentation/internals/contributing/coding-style.rst > in a few different places, and we should probably discuss it more > extensively than just a bare ack. > sure. I missed updating the line in the docs. and yes please, let's do the discussion as needed. I wanted to get it started so this is why I sent this patch. i'll send v2 with update to the docs. thanks.
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index fc9e20bf1b5f..80ee6d827567 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -287,8 +287,8 @@ def pointer_whitespace_check(line): def line_length_check(line): """Return TRUE if the line length is too long""" - if len(line) > 79: - print_warning("Line is %d characters long (recommended limit is 79)" + if len(line) > 100: + print_warning("Line is %d characters long (recommended limit is 100)" % len(line)) return True return False
This is to match a recent kernel checkpatch change that also increased it to 100 line length. Signed-off-by: Roi Dayan <roid@mellanox.com> --- utilities/checkpatch.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)