Change in coreboot[master]: Documentation: Codify some guidelines for headers and chain-including
Attention is currently required from: Julius Werner. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50247 )
Change subject: Documentation: Codify some guidelines for headers and chain-including ......................................................................
Patch Set 2: Code-Review+1
(4 comments)
Patchset:
PS2: In general, I'm good with everything here, but we might want to add something like checkpatch that just comments on issues for a while until things are following the guidelines and can be enforced with automated tools.
File Documentation/coding_style.md:
https://review.coreboot.org/c/coreboot/+/50247/comment/35736dce_8137a85f PS2, Line 841: alphabetical order. I wouldn't say that this is codifying what's already being done. I agree that it's nice, but right now it's definitely not like this across the codebase.
Also, does this mean mixing #include <file.h> with #include "file2.h" or would the local file includes still generally come after?
https://review.coreboot.org/c/coreboot/+/50247/comment/938cb7a6_bd3d7fce PS2, Line 848: : Files should generally include every header they need a definition from : directly Is there a good tool to tell if everything is supplied by a directly included header or by something included indirectly?
https://review.coreboot.org/c/coreboot/+/50247/comment/701ab5e5_d991e04a PS2, Line 850: Excepted from : this are certain headers that intentionally chain-include other headers : which logically belong to them and are just factored out into a separate : location for implementation or organizatory reasons. Maybe we want to build a list of these allowed chained headers to head off any arguments? What's logical to one person may not be logical to another.
That wouldn't need to be included here (or now), but having something in the future might be good.
-
Martin Roth (Code Review)