Hi Patrick, Thanks for setting this up! I think it's very important that we give everyone a voice in these matters. Let me link some old discussions in case people want more context:
https://mail.coreboot.org/hyperkitty/list/[email protected]/thread/4FAD25MJPI2XSSWHDDI5CSJR6SKCRFWW/#FDPB2PVVV7AB2CPJSGX4S2SFA4WNURN4 https://review.coreboot.org/c/coreboot/+/27533 https://mail.coreboot.org/hyperkitty/list/[email protected]/thread/DXEYNXR37SW4H5QLREDFTOZRUUBJ4QLW/#3ZLYUYAP6WMXIPADPR5WUSBRASIKODD3 One thing I'm missing is an option to maintain clang-format as an optional pre-submit hook that people can choose run over only the lines their patch is touching if they want to, like I suggested in that thread above. I think if we have so many options per question anyway, that one would have been nice to add. Maybe next time we can post the draft questions and give people a chance to comment first. But the most important thing is that we're doing it at all. From: Patrick Georgi via coreboot <[email protected]> Date: Tue, May 14, 2019 at 11:11 AM To: coreboot > 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 <[email protected]> > Kyösti Mälkki <[email protected]> > 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 > _______________________________________________ > coreboot mailing list -- [email protected] > To unsubscribe send an email to [email protected] _______________________________________________ coreboot mailing list -- [email protected] To unsubscribe send an email to [email protected]
