qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] scripts/checkpatch.pl: Modify the line length limit of the c


From: Markus Armbruster
Subject: Re: [PATCH] scripts/checkpatch.pl: Modify the line length limit of the code
Date: Fri, 06 Nov 2020 17:08:19 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 6 Nov 2020 at 14:08, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> Can we keep the error please? Maybe 132 is the next display logical
>> limit once we increased the warning from 80 to 100.
>>
>> I understand hardware evolved, we have larger displays with better
>> resolution and can fit more characters in a line.
[...]
>
> Personally I just don't think checkpatch should be nudging people
> into folding 85-character lines, especially when there are
> multiple very similar lines in a row and only one would get
> folded, eg the prototypes in target/arm/helper.h -- some of
> these just edge beyond 80 characters and I think wrapping them
> is clearly worse for readability.

The warning's intent is "are you sure this line is better not broken?"
The problem is people treating it as an order that absolves them from
using good judgement instead.

I propose to fix it by phrasing the warning more clearly.  Instead of

    WARNING: line over 80 characters

we could say

    WARNING: line over 80 characters
    Please examine the line, and use your judgement to decide whether
    it should be broken.

>                                   If we don't want people
> sending us "style fix" patches which wrap >80 char lines
> (which I think we do not) then we shouldn't have checkpatch
> complain about them, because if it does then that's what we get.

I think that's throwing out the baby with the bathwater.

checkpatch's purpose is not guiding inexperienced developers to style
issues they can fix.  It is relieving maintainers of the tedium of
catching and explaining certain kinds of issues patches frequently have.

Neutering checks that have led inexperienced developers to post less
than useful patches may well relieve maintainers of having to reject
such patches.  But it comes a price: maintainers and contributors lose
automated help with checking useful patches.  I consider that a bad
trade.

We may want to discourage inexperienced contributors from sending us
style fix patches.  Fixing style takes good taste, which develops only
with experience.  Moreover, fixing up style builds only little
experience.  At best, it exercises "configure; make check" and the patch
submission process and running "make check").  There are better ways to
get your feet wet.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]