All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Potapenko <glider@google.com>
To: miguel.ojeda.sandonis@gmail.com
Cc: christian.brauner@ubuntu.com, clang-built-linux@googlegroups.com,
	 joe@perches.com, linux-kernel@vger.kernel.org,
	torvalds@linux-foundation.org,
	 Alexander Potapenko <glider@google.com>,
	Yury Norov <yury.norov@gmail.com>
Subject: Re: [PATCH] .clang-format: update column limit
Date: Mon, 18 Dec 2023 11:18:16 +0100	[thread overview]
Message-ID: <20231218101816.508663-1-glider@google.com> (raw)
In-Reply-To: <CANiq72m1E1=7yVz=jrDJWBtLZzDsi0ygFvZaHrOk_EbKdbf38A@mail.gmail.com>


Hi folks,

this came up in another code review
(https://lore.kernel.org/lkml/CAG_fn=WcrNqV4burBRPZZwoBLwgia7kerZ8g2vV5spzWF=houQ@mail.gmail.com/),
so I thought maybe it's time to revisit changing of ColumnLimit for clang-format?

I ran the script Joe provided, and there's a noticeable drift towards the longer lines in the past
three years:

 201789 78 87.16
 201579 79 87.77
 138155 80 88.19
  55886 81 88.35
  42656 82 88.48
  39539 83 88.60
  36490 84 88.71
  37856 85 88.83
  68131 86 89.03
  29352 87 89.12
  23277 88 89.19
  26902 89 89.27
  21405 90 89.34
  44506 91 89.47
  70788 92 89.69
  13416 93 89.73
  16650 94 89.78
  10872 95 89.81
   9786 96 89.84
  12170 97 89.88
   9139 98 89.90
   8516 99 89.93
   7008 100 89.95
   6058 101 89.97
   4692 102 89.98

At our team we try to run clang-format on all newly written code, which to great extent helps
to avoid style-related bikeshedding. I changed the local limit in my checkout and reflowed
mm/kmsan/*.c and mm/kfence/*.c, and they actually look better than before.

This is sure not enough to justify the 80->100 change, but as far as I can tell right now
reviewers are not actively paying attention at the lines that are shorter than 100 columns,
even if those exceed the 80-column limit. So even if all clang-format users switch to 100 columns
locally this wouldn't cause objections as long as checkpatch.pl is happy.
On the other hand, the readability of the code they produce is likely to increase.

Given that nothing changes for people who don't use clang-format, maybe having the limit conform
to what checkpatch.pl wants is not a bad idea after all?

      reply	other threads:[~2023-12-18 10:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-10 12:51 [PATCH] .clang-format: update column limit Christian Brauner
2020-06-10 15:55 ` Miguel Ojeda
2020-06-10 15:58   ` Nathan Chancellor
2020-06-10 16:23     ` Sedat Dilek
2020-06-10 17:13 ` Joe Perches
2020-06-10 17:32   ` Christian Brauner
2020-06-11 10:03   ` Miguel Ojeda
2020-06-11 10:36     ` Joe Perches
2020-06-11 11:54       ` Miguel Ojeda
2020-06-11 16:22         ` Joe Perches
2020-06-11 18:51           ` Miguel Ojeda
2020-06-11 19:26             ` Joe Perches
2020-06-22  0:03               ` Miguel Ojeda
2023-12-18 10:18                 ` Alexander Potapenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231218101816.508663-1-glider@google.com \
    --to=glider@google.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.