qemu-trivial
[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 14:07:02 +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 06:15, Gan Qixin <ganqixin@huawei.com> wrote:
>>
>> Modify the rule that limit the length of lines according to the following 
>> ideas:
>>
>> --add a variable max_line_length to indicate the limit of line length and 
>> set it to 100 by default
>> --when the line length exceeds max_line_length, output warning information 
>> instead of error
>> --if/while/etc brace do not go on next line whether the line length exceeds 
>> max_line_length or not

The commit message fails at explaining *why*.

A controversial change like this should not be committed without proper
rationale.

>> Signed-off-by: Gan Qixin <ganqixin@huawei.com>
>> ---
>>  scripts/checkpatch.pl | 18 +++++-------------
>>  1 file changed, 5 insertions(+), 13 deletions(-)
>
> For the code changes
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> but we also need to update our coding style documentation
> to match. I'll send out a patch with some proposed text.

I disagree with the coding style change.

The current "warn at 80, error at 90" is a compromise.  It's the result
of a lengthy argument.  Why reopen it?

> Side note: the kernel version of this checkpatch change
> (kernel commit bdc48fa11e46f867) suppresses all line-length
> warnings for the "--file" use case. Do we care about that?

The kernel patch at least has a decent commit message.




reply via email to

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