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