Re: [Qemu-devel] [PATCH 2/4] CODING_STYLE, checkpatch: update line length rules

[Thomas Huth <thuth@redhat.com>]

On 07/09/15 16:22, Paolo Bonzini wrote:
> 
> On 07/09/2015 15:18, Thomas Huth wrote:
>> On 07/09/15 11:53, Paolo Bonzini wrote:
>>> Line lengths above 80 characters do exist.  They are rare, but
>>> they happen from time to time.  An ignored rule is worse than an
>>> exception to the rule, so do the latter.
>>>
>>> Based on remarks from the list, make the preferred line length
>>> slightly lower than 80 characters, to account for extra characters
>>> in unified diffs (including three-way diffs) and for email quoting.
>>>
>>> Checkpatch has some code to detect doc comments that doesn't apply
>>> to QEMU; the usual limits apply even for doc comments in our case.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  CODING_STYLE          | 13 ++++++++++---
>>>  scripts/checkpatch.pl | 21 +++++++++++++++------
>>>  2 files changed, 25 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/CODING_STYLE b/CODING_STYLE
>>> index 3c6978f..34d5526 100644
>>> --- a/CODING_STYLE
>>> +++ b/CODING_STYLE
>>> @@ -31,14 +31,21 @@ Do not leave whitespace dangling off the ends of lines.
>>>  
>>>  2. Line width
>>>  
>>> -Lines are 80 characters; not longer.
>>> +Lines should be 76 characters; try not to make them longer.
[...]
>> Sorry, this is _ugly_! First, the limit in QEMU has always been 80
>> columns in the past, thus we've got tons of code that is written with 80
>> columns already, so you suddenly even can not even copy-n-paste / move
>> around code anymore that was valid before?!? That'll be a PITA since you
>> have to reformat all patches that move code around - and it will also be
>> a PITA for all reviewers since checking whether old code matches new
>> code becomes more difficult.
[...]
> Some other interesting data:
> 
> - However, only 605 files have 5 or more lines with 77+ columns, so the
> odds of warnings after copy-n-paste are pretty slim.

Apart from copy-n-pasting, there is also the problem that you can run
"checkpatch.pl -f" on a whole file ... it would also be ugly to suddenly
have (much) more warnings here.

> Based on the above data, I suggest having 79 columns as the limit (with
> the "BSD exception" unfortunately).  This still makes it possible to
> watch diffs on an 80-column terminal without wrapping.

79 sounds at least better to me than 76. So if you really want to change
the limit, please go for 79 instead of 76.

 Thomas