Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31651 )
Change subject: lint/clang-format: set to 96 chars per line ......................................................................
Patch Set 2:
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.