Eelco Chaudron <[email protected]> writes:

> On 27 Sep 2023, at 15:05, Ilya Maximets wrote:
>
>> On 9/27/23 14:34, Roi Dayan wrote:
>>>
>>> On 18/06/2020 21:17, 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 <[email protected]>
>>>> Reviewed-by: Simon Horman <[email protected]>
>>>> Acked-by: Aaron Conole <[email protected]>
>>>> ---
>>>>
>>>> Notes:
>>>>     v2
>>>>     - update docs
>>>>     - add ack from Simon and Aaron
>>>>
>>>>  Documentation/internals/contributing/coding-style-windows.rst | 2 +-
>>>>  Documentation/internals/contributing/coding-style.rst         | 2 +-
>>>>  Documentation/internals/contributing/documentation-style.rst  | 2 +-
>>>>  utilities/checkpatch.py                                       | 4 ++--
>>>>  4 files changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/Documentation/internals/contributing/coding-style-windows.rst 
>>>> b/Documentation/internals/contributing/coding-style-windows.rst
>>>> index 1edf2285ff3c..a72d9fb619e7 100644
>>>> --- a/Documentation/internals/contributing/coding-style-windows.rst
>>>> +++ b/Documentation/internals/contributing/coding-style-windows.rst
>>>> @@ -38,7 +38,7 @@ kernel/driver code.  They are noted as follows:
>>>>  Basics
>>>>  ------
>>>>
>>>> -- Limit lines to 79 characters.
>>>> +- Limit lines to 100 characters.
>>>>
>>>>    Many times, this is not possible due to long names of functions and it 
>>>> is
>>>>    fine to go beyond the characters limit.  One common example is when 
>>>> calling
>>>> diff --git a/Documentation/internals/contributing/coding-style.rst 
>>>> b/Documentation/internals/contributing/coding-style.rst
>>>> index f70f783add08..62adfe186347 100644
>>>> --- a/Documentation/internals/contributing/coding-style.rst
>>>> +++ b/Documentation/internals/contributing/coding-style.rst
>>>> @@ -43,7 +43,7 @@ The following GNU indent options approximate this style.
>>>>  Basics
>>>>  ------
>>>>
>>>> -- Limit lines to 79 characters.
>>>> +- Limit lines to 100 characters.
>>>>
>>>>  - Use form feeds (control+L) to divide long source files into logical 
>>>> pieces. A
>>>>    form feed should appear as the only character on a line.
>>>> diff --git a/Documentation/internals/contributing/documentation-style.rst 
>>>> b/Documentation/internals/contributing/documentation-style.rst
>>>> index deb07d9f5dde..8b158de2e188 100644
>>>> --- a/Documentation/internals/contributing/documentation-style.rst
>>>> +++ b/Documentation/internals/contributing/documentation-style.rst
>>>> @@ -64,7 +64,7 @@ Many of the basic documentation guidelines match those 
>>>> of the
>>>>    Sphinx extensions can be used, but only for documentation in the
>>>>    ``Documentation`` folder.
>>>>
>>>> -- Limit lines at 79 characters.
>>>> +- Limit lines at 100 characters.
>>>>
>>>>    .. note::
>>>>
>>>> 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,
>>>
>>> This is an old patch that was never taken or even started a real discussion.
>>> Can we get to it again?
>>> 79 character limit is an old style used for small terminal monitors which
>>> less reflects today.
>>
>> Hi, Roi.
>>
>> The current limit has an implicit value of restricting the nesting levels
>> in the code.  Also, the comparison with a Linux kernel is not fully correct.
>> First, the kernel is using 8 spaces wide tabs that make lines much longer.
>> Secondly, kernel didn't actually change the recommended line length in the
>> style, they only changed the threshold at which checkpatch starts to warn.
>> See:
>>  
>> https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings
>>
>> So, I'm not sure why we need to change that.  For me personally, I do use
>> multiple narrow terminal windows fairly frequently while working on the code.
>>
>> Another concern is narrow windows with enlarged font size during screen
>> sharing and similar cases.
>
> I’ve never found it an issue trying to wrap my code to 80 chars, and
> like Ilya I find it useful when browsing code that I can have multiple
> sources open in parallel to compare/search (and yes I have a wide 32”
> monitor :).
>
> So unless there is a real use case for moving to 80+ chars, I would suggest 
> keeping it at 80.

I think one case we might consider would be to allow strings that would
need line wrap after 80 chars.  I do like when strings in the code can
be grep'd easily.  That said, I'm not aware of many cases currently in
the code where this is a problem - and the coding style doesn't
currently address this case anyway.

As usual, we can always look at the code and if it is nicer with longer
lines we can just ignore the warning.

> Cheers,
>
> Eelco

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to