Votes on copyright lines, line lengths and automatic formatting - coreboot

14 May 2019


      Hi everybody,
The project leadership asked me to get the developer community's
opinion on a couple of coding style related issues that were under
discussion for a long time without clear resolution.
To that end I just setup three elections on
https://civs.cs.cornell.edu. They're ranked votes using the Condorcet
method with a single winner per question. Voting ends on end of May 21
AoE (https://en.wikipedia.org/wiki/Anywhere_on_Earth).
The voting population was determined by Martin and covers everybody
who made 10 contributions (changes, review flags) on
review.coreboot.org over the last 2 years.
These eligible developers should have received an email from me last
week to introduce the process and three emails today with their
personalized link to the elections, one for each question. If you
think you're eligible but didn't get these emails, please reach out to
me directly.
For reference (but there won't be a vote here on this list!), the
questions posed are:
# How to handle copyright notices
The coreboot project has had issues with copyright notices for a long
time, and we’d like to start to address it.  We currently don’t have
any guide on when it’s ok to add a copyright, and when it isn’t. This
and other issues like refactoring patches in gerrit lead to things
like:
* Copyright on blank files
* Adding a copyright line when deleting code
* Adding a copyright line for trivial changes such as spelling fixes
The copyright laws around the world differ, but it seems like most of
them agree that copyright is specifically for acts of creativity.
None of the above examples are creative, so that would exclude them
from copyright protection.
Another issue is that we’d rather not have an ever-growing list of
copyright notices at the top of each file.  This has been addressed in
other projects by having an AUTHORS file, and it’s been suggested that
coreboot should follow this.
The AUTHORS file would contain the list of copyright holders, based on
the existing copyright lines.
# This is the list of coreboot authors for copyright purposes.
#
# This does not necessarily list everyone who has contributed code, since in
# some cases, their employer may be the copyright holder.  To see the full list
# of contributors, see the revision history in source control.
Ronald G. Minnich rminnich@gmail.com
Kyösti Mälkki kyosti.malkki@gmail.com
SUSE LINUX AG
coresystems GmbH
All existing copyright notices would be replaced with text  “Copyright
2019 The coreboot project Authors.”
Arguments for replacing existing copyright lines with the AUTHORS file:
* This resolves many of the current issues with copyright notices.
 * Without copyright notices in each individual file, once a name is
in the AUTHORS file, the copyright holder doesn’t have to update it
again.
 * The headers won’t grow because there’s just one static line.
* Copyright dates won’t have to be updated in multiple files anymore
Arguments for leaving the copyright lines:
* A single AUTHORS file doesn’t capture the amount of work that people
have put into the codebase.  The copyright line allows people to see
their contributions to coreboot.
Please rank these choices:
* Create an authors file and remove all existing copyright lines
* Create an authors file but leave existing copyright lines
* Don’t create an authors file or change existing copyright lines
# Line length
There have been arguments recently that the line length is a bit too
restrictive and we should change the coding style to allow for longer
lines.
We currently have line length exceptions for comments and print format strings.
Arguments for longer line lengths:
* We have large monitors and can easily see more than 80 characters now.
* It’s better to just make the code look good than to worry about
breaking the line up because it’s 81 characters long.
* With long Kconfig symbol names and longer, more descriptive variable
names, lines have gotten longer without changing any code complexity.
Arguments for keeping 80 characters:
* If the code needs to go past 80 characters, the function is probably
too complex.
* The linux kernel has an 80 character limit, and we should maintain that.
* With the existing line length exceptions, there shouldn’t be a problem.
* People already have their editors set up for 80 characters and it
would be a pain to change them just for coreboot’s new settings.
Argument for 96 characters:
* This is intended to be a compromise between 80 characters and even
longer line lengths.  The idea here is that it’s 2 tabstops + 80 text
characters.
Argument for no strict line length:
* Nobody is actually advocating for super long lines - it’s just about
making the code look good without actually having to worry about the
line length.
* We can trust coreboot developers to be reasonable and we can catch
people doing stupid things in the code reviews when they’re not.
Rank these options:
* 80 character lines
* 88 character lines
* 96 character lines
* 120 character lines
* 132 character lines
* No strict length limit
# Automatic code formatting
As people have worked in other languages like Go which has strictly
enforced automatic formatting, it has gained advocates.  We’re trying
to decide whether to require that code be formatted by clang-format
before submitting to gerrit or to update the formatting automatically
every once in a while.
Arguments for automatic formatting
* Currently most comments in gerrit are about style, not substance.
If the code is automatically formatted, we can focus on the contents
of the code, not the style of the code.
* If manual formatting is important in a location, the section can be
excluded from clang-format, so this gives us more options.
Arguments against automatic formatting
* Different versions of clang-format produce different output - it’s
not the same every time.
* Code will sometimes end up being less readable after running through
clang-format.
* clang-format isn’t smart enough
Rank these options:
* Submitters should run clang-format before submitting a patch and it
should be rejected in gerrit if it’s not formatted properly with an
error telling them how to format it.
* Code should just get automatically reformatted on a regular basis.
* coreboot should stick with manual formatting.
Regards,
Patrick
-- 
Google Germany GmbH, ABC-Str. 19, 20354 Hamburg
Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado