On 09/06/2022 10:56, Peter Maydell wrote:
On Mon, 6 Jun 2022 at 15:34, Lucas Mateus Castro(alqotel)
<lucas.ara...@eldorado.org.br> wrote:
---
Currently there's a disagreement between the checkpatch code and the
documentation, this RFC just changes the checkpatch to match the
documentation.
But there was a discussion in 2020 as the best way to deal with this,
some alternatives mentioned are: change the warning to remind people to
not blindly wrap just because of the warning, change to warn at 90 columns
(which would mean changing the column limit for the error as well). If any
of those are preferred I'll send a next version updating the documentation
as well as changing checkpatch.pl to the preferred behavior.
The reason the code doesn't match the style docs is partly
my fault I guess. The style docs were updated with
commit a998de0dcd ("CODING_STYLE.rst: Be less strict about 80 character limit");
that commit message says "this goes with the checkpatch changes to warn at
100 characters rather than 80", but we never committed the checkpatch
changes. Those were this patch:
https://lore.kernel.org/qemu-devel/20201105154208.12442-1-ganqi...@huawei.com/
(which started the discussion out on an awkward footing by not including
the rationale in its commit message; it was prompted by a discussion on
a previous patchset:
https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg05653.html
Equivalent kernel checkpatch.pl loosening:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
Sorry I should've added those links to the commit message to update
anyone getting in this discussion now, thanks for adding them.
Anyway, I think the main objector last time around was Markus, and
there's kind of an unresolvable difference of views here
My main idea with the RFC is mostly to revive the discussion to fix the
difference between the style docs and the code, so I hope it's not
completely unresolvable
Also my emails to Markus are bouncing back so hopefully he finds this
patch in the qemu list.
(1) I want checkpatch not to warn about line lengths that in fact
we'd be happy to include in the tree (because it's noise, and it
pushes people into wrapping the cases which the style guide describes
as "awkwardly wrapped" and better not wrapped, to silence the warning),
so I don't want checkpatch to even warn on less than 90 or 100 chars.
This is effectively also the position that the Linux kernel
checkpatch takes these days.
Philippe Mathieu-Daudé talked about keeping some error to avoid overly
long lines, so that part I added to my patch.
(2) Markus (as I understand his point of view from the 2020 thread)
wants checkpatch to warn about anything over the "this is definitely
fine" 80 column mark, so that developers are reminded that
they might want to make a judgement call there.
The coding-style text reflects my point of view, because I
wrote it, and therefore personally I'm happy to update checkpatch
to match it :-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d900d18048..2c2d7b31ab 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1639,12 +1639,12 @@ sub process {
if ($line =~ /^\+/ &&
!($line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) &&
!($rawline =~ /^[^[:alnum:]]*https?:\S*$/) &&
- $length > 80)
+ $length > 100)
Gan Qixin's patch has the advantage of putting the max length into
a variable rather than continuing to hardcode it.
I can send a v2 with these changes if this is the way we're heading, or
maybe we could use Gan Qixin's patch (although a more detailed commit
message might be desired)
thanks
-- PMM
--
Lucas Mateus M. Araujo e Castro
Instituto de Pesquisas ELDORADO
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
