[coreboot] [RFC] Deciding on style for multi-line comments

[Martin Roth]

Can we please just decide on some tool and setting that we can run all the
changes through that will "correctly" format it so we can stop arguing
about the format?


Martin

On Thu, Aug 25, 2016 at 12:59 PM, Julius Werner <jwerner at chromium.org>
wrote:

> First of all, let's dispel the notion that there was one "correct" and
> one "wrong" comment style in coreboot right now.
>
> If we count the amount of multi-line comments in the concise style
>
> > coreboot $ egrep '^\s*/\* [^/]+$' src/ | wc
> > 5230 50584 491997
>
> against the amount of multi-line comments in the verbose style
>
> > coreboot $ egrep '^\s*/\*$' src/ | wc
> > 16103   18607  827285
>
> and subtract the counts from the FSF license header from that, which
> uses the verbose style and is just copy&pasted into every file
>
> > coreboot $ grep 'FITNESS FOR A PARTICULAR PURPOSE' src/ | wc
> > 11857  133958 1424055
>
> we find that the concise style is actually more common in coreboot
> right now. I find the idea of overruling thousands of instances of
> established practice on behalf of some ancient Wiki text that the
> author obviously didn't even read before copy&pasting it in there
> kinda ridiculous.
>
> Secondly, Linux cannot even decide for itself which comment style they
> prefer (as exemplified by their net/ directory exemption, which they
> have maintained for years despite the fact that enforcing different
> styles in different parts of the repo is truly crazy), so I don't
> think "do what Linux does because Linux is obviously always right"
> makes much sense here either. As far as I can tell, they also don't
> have any reasonable justification for their decision other than "it's
> what Linus (and the net/ maintainer) like". If we really wanted to
> blindly follow the Linux way here, we'd have to pick one directory
> under coreboot/src/ that arbitrarily enforces a different style than
> the others for no reason whatsoever.
>
> Look, I'm not saying that we should only be using the concise style.
> I'm just saying that both styles have their valid uses and we
> shouldn't be forcing one over the other in every possible case. Yes,
> the verbose style separates comments and makes them stand out better,
> and sometimes that's a good and desirable thing! If you want to put in
> a big explanation that applies to a whole code block or something, it
> may well be the better choice.
>
> But in other cases you maybe just want to explain a tricky detail
> about a single statement in a way that you can't quite fit it on one
> line, and forcing people to waste four lines and disrupt the whole
> flow of the reader at a point where this wouldn't make sense is not a
> good decision in that case. It would be similar to decreeing something
> like "there may be at most three statements in a row without a blank
> line in between them"... yes, blank lines often increase readability
> and are encouraged by our coding style, but only in places that make
> sense in the context of the code. Screen real estate is a real thing
> and can make an already complicated piece of code way harder to
> understand because you can't fit it all on one screen anymore. We'd be
> actively making code less readable by forcing such an inflexible
> style.
>
> All I'm saying is that the programmer writing the comment and the
> surrounding code probably has a better idea about which amount of
> blank space would make it most readable and understandable than some
> outdated Wiki page, so this decision should be left to them. We're
> writing comments with the explicit intention of helping others
> understand our code, after all, so we're naturally going to write them
> with the style that works best for each individual case. Forcing the
> verbose style everywhere would just cause people to unnecessarily
> shorten comments to one line when a big 4+ multi-line comment would be
> too disruptive to readability, and thus decrease the overall quality
> of our comments.
>
> --
> coreboot mailing list: coreboot at coreboot.org
> https://www.coreboot.org/mailman/listinfo/coreboot
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20160826/cf66eccb/attachment.html>