Will try to keep this short.

>> so you can upload a patch that sets "ColumnLimit: 80" and I'll happily +2 that.
> I thought the rough consensus was "80 columns except where it _really_, _really_ makes sense"? In that case ColumnLimit: 80 (which sets a hard boundary) is of no use.

The problem I see with clang-format is that it only knows
a simple hard limit. What does a rule like "2 tabs + 80"
mean for a tool like clang-format? I couldn't decide. OTOH,
it seems easy to adapt checkpatch for such a rule.

>> As far as I am aware we consider clang-format an option for developers who chose to use it, and there are no plans to change that.
>
> The mere existence of util/lint/lint-*022* disagrees with "just an option" (and note, the only I change I did in that space was to move it from experimental to stable, but util/lint is the wrong place for optional non-rules.)

I have to admit, I don't understand our current setup.
Some scripts seem to assume that you opt in for clang-
format others don't?

>> I would likely be opposed to it if there were (and I assume others would be as well, e.g. it sounds like Nico is not a fan either).
>
> As Ron reports from other projects, nobody is happy with it before, while everybody seems to accept it afterwards.

What everybody accepts? Clearly in times of political
apathy, the ruler wins. But what does that have to do
with this discussion?

My biggest fear is that our review quality could
decline. It's not measurable anyway. But to me it
seems that longer lines are more eye-weary. In case
we have a poll on the matter, I'd love to see an
estimate how much time the voter spends on reading
code.

>> (I am not sure what setting ColumnLimit: 0 *and* removing the checkpatch check is supposed to accomplish... if you're trying to say we should remove the line length limit altogether, then no, I'm obviously not in favor of that either.)
>
> ColumnLimit: 0 doesn't mean that clang-format makes every line as long as possible, but that it leaves lines alone that it doesn't touch and otherwise creates lines as long as necessary. A concession to that "80 columns except where it makes sense" thing.

That actually doesn't sound bad.

Since apparently the strongest opinions on that (not necessarily a "majority" by any metric) are for artisanal source code, I guess the only resort we have is to drop both checkpatch and clang-format and continue to rely on the services of Paul and Elyes to keep us in line with a style guide that nobody really follows anyway. I'll prepare a patch.

I always thought I have the strongest opinion :-P If
there is a little consensus, I could try to adapt
checkpatch to an indentation + x rule. But maybe we
should first decide if we want clang-format.

View Change

To view, visit change 31651. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9293a69d1bea900da36501cde512004d0695ad37
Gerrit-Change-Number: 31651
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Julius Werner <jwerner@chromium.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-CC: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer@coreboot.org>
Gerrit-CC: ron minnich
Gerrit-Comment-Date: Thu, 14 Mar 2019 16:13:19 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment