Hello,
On Sat, Mar 16, 2019 at 4:32 PM Nico Huber nico.h@gmx.de wrote:
Hey folks,
I overeagerly reviewed and submitted a change[1] lately, that set the column limit for our C code to 96. My reasoning was that we already live a "soft" limit of 80 chars and that tools shouldn't complain about every single 8x-chars line (personally, I find this quite annoying during review). What I missed is that people use the limit to format code automatically, resulting in less line breaks, even if they'd make sense for readability.
I find tools complaining on 8x-char lines rather annoying as well. Especially when a bunch of lines get a warning each, and gerrit's web interface slows down to a complete halt.
Do we want to enforce a single editor / IDE + configuration for coreboot contributions? For instance, Vim is quite configurable and helpful when writing code. This could make all tools for later processing unneces- sary.
IMHO this will cause more trouble than it will solve. I use a mixture of nvim and Geany so I wouldn't mind that much, but other people may use a completely different editor/IDE and I guess they would be rather reluctant to change just to contribute to coreboot.
Do we want to enforce a single tool, e.g. clang-format, that does the job for us after editing a source file?
The above, even if that implies a new coding style that we might not be used to?
One thing that I *really* hate w.r.t. tools is when they want to do something in a specific way and don't allow manual changes. IMHO this doesn't sound like a good idea.
However, I understand that manually maintaining everything in shape is a daunting task, and why some people would want an automated tool.
Do we want a combination of such a tool and check-patch? For instance, clang-format has a feature to ignore broken lines. This could then be handled by check-patch, to allow more complex rules.
Do we just want to keep check-patch and let authors / their editors format the code?
Isn't this what we have now? IMHO I find checkpatch warnings useful, though some code style things such as CamelCase don't get caught at all by it, and are sometimes merged in.
Do we want to rely (solely) on reviewers for format checking?
I believe some of the current checks are rather useful, so I would leave them in place. Some other style nits are better judged by humans, though.
Do we want to encourage reviewers to educate their fellows on code style (for instance, wrt. line length, less indentation levels, shorter, more meaningful identifiers, etc.)?
If we get to agree on one code style, yes.
Nico
Do note this is my personal, uneducated opinion on the matter.
Best regards,
Angel Pons