-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CODING_CONVENTIONS.md: Add preprocessor directive formatting #20866
CODING_CONVENTIONS.md: Add preprocessor directive formatting #20866
Conversation
Note: I'm not adding an "API change" label, as this is intended to really just document what is current policy anyway, rather than changing things. It would be good to confirm that this is indeed the implicit policy now, though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing that down! I agree with the reasoning and am in favor of adding it, but don't feel in the position to acknowledge this being an implicit convention in the past.
Let's give this PR a few days to gather feedback. |
👍 |
The convention described, seems to match that of the newer code that I have looked at. I agree that the convention is a logical choice. Also the wording and examples seem clear to me. I don't seem to have sufficient permission to approve, but I would if I could. |
This explicitly spells out what informally has been the coding convention for some time on preprocessor directives, making it more transparent and easier to find. This is particularly useful as the code base has at least three different styles. Deducing what actually is the current policy would require a details look at how the style has changed over time, rather than being obvious from a quick `grep` over the code base.
squashed |
869b56d
to
f431aab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this was enough time for others to raise their voice :)
Contribution description
This explicitly spells out what informally has been the coding convention for some time on preprocessor directives, making it more transparent and easier to find.
This is particularly useful as the code base has at least three different styles. Deducing what actually is the current policy would require a details look at how the style has changed over time, rather than being obvious from a quick
grep
over the code base.Testing procedure
Check that the wording is clear and easy to understand and that the policy matches what we actually do. (E.g. look at ztimer, as this is a rather new component that has received lots of reviewing.)
Issues/PRs references
#20864 (comment)