Attention is currently required from: Martin L Roth, Patrick Georgi, Paul Menzel.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75879?usp=email )
Change subject: Docs/contrib/coding_style: Document the preference for if() vs #if ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
File Documentation/contributing/coding_style.md:
https://review.coreboot.org/c/coreboot/+/75879/comment/88b3c88a_fbd99ac1 : PS1, Line 539: ifdef Should we go further and say that #ifdef is just forbidden entirely? With the way CONFIG() works in coreboot it should basically always be an error. The only thing where it still seems to be used is `#ifdef __SIMPLE_DEVICE__` and it would probably be better if we refactored that to work more like the other stuff in rules.h (e.g. `#if ENV_SIMPLE_DEVICE`).
Mixing #if and #ifdef can easily lead to mistakes when you accidentally use #ifdef on something that's defined to 0 when it is off, so I think it would be nice to outlaw (and linter-enforce) #ifdef entirely and use the more visually obvious `#if defined()` in places where we actually intentionally want to check for the definition of a macro (e.g. in rules.h itself).
https://review.coreboot.org/c/coreboot/+/75879/comment/e21cdf97_5438ef9f : PS1, Line 547: shouldn't nit: maybe say "should usually not be a need" because there are some rare cases where there really is a need (e.g. https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src... where it includes separate headers defining the same function depending on Kconfig... not saying that was necessarily the best design to begin with, but that's what we have there at the moment).