These "other projects" would be mostly Linux, since that's the style we're closest to: try getting our code into a GNU project (for example) without reformatting it entirely.

But we are not Linux. IS_ENABLED() has different semantics than Linux's, oh, and we use CONFIG() now. Our Kconfig has different semantics than Linux's. We use volatile, which is frowned upon in Linux.

This was just part of the argument why some people prefer 80 characters, and why 96 is not "a compromise". No, we are not Linux, and we don't have to be, but many people working on this project come from Linux (or U-Boot or any of the other dozens of projects that copy its style) and many of them prefer to have more consistent style between the different projects they work on and sometimes port code back and forth between.

I am not saying that we have to be Linux. I am saying that you're making a sweeping change to the project just because you felt like it without taking any of the objections that you knew were there into account or even trying to discuss the issue again first. Code style is always subjective, but that means that it should only be changed by consensus or at least by clear majority opinion. You don't have that here, you're just doing whatever you want and completely ignoring the fact that there are hundreds of other developers on this project who may or may not agree with your subjective change. *That* is what I'm opposed against!

This commit doesn't change everything from 80 to 96: it changes checkpatch from 80 to 96 and clang-format from "whatever" to 96. A plain revert would bring that inconsistency back and so I'm strongly opposed to that.

Uhh... sorry, what? You yourself just submitted CB:31771 which literally says in the subject "our coding style now allows 80 + 2*8 columns in a line", citing this patch as justification. I am arguing against and asking you to revert both of these, obviously.

If you want to align clang-format to the coding style we use in coreboot, the status quo on that clearly is 80, so you can upload a patch that sets "ColumnLimit: 80" and I'll happily +2 that. If you instead want to change the line length of our coding style, please be respectful of the fact that this is a very controversial topic that affects every single developer on the project, many of whom have an opinion about it, and you can't just change it willy-nilly without making some effort to confirm that most of the community is in favor of your change first.

But how about this: We modify checkpatch so we can disable its coding style checks (as opposed its more useful tests about CONFIG(), for example) and do that for everything that is covered by clang-format (as defined in .clang-format-scope).

That also neatly solves the remaining consistency issues (see the checkpatch warnings in CB:31652 that complain about code as produced by clang-format) by having one tool decide on style instead of two tools fighting it out.

With that we can go back to clang-format's ColumnLimit: 0 (ie its "whatever" option) for the code it handles and checkpatch back to 80 for the code that it doesn't.

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. 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). But that is a separate topic from the line-length discussion, and as mentioned above, I am perfectly fine with matching the line length limit we have in .clang-format if that's what you want to do. (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.)

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: Stefan Reinauer <stefan.reinauer@coreboot.org>
Gerrit-CC: ron minnich
Gerrit-Comment-Date: Mon, 11 Mar 2019 20:18:41 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment