[RFC] Defining a code standard.

[jalinei]

Is your feature request related to a problem? Please describe. Currently each modules has been written by different authors and code style is not uniform. I propose that we tend to follow Zephyr coding style as our project relies on that RTOS.

I'm creating this RFC as this applying this change requires tree-wide modifications.

Describe the enhancement you'd like Adding a clear code standard defined in a file named CONTRIBUTING.md. I propose we add a clang-format file to be able to format our code using included extensions in vscode.

To enforce these guidelines I propose to add checkpatch tool in CI for future PR using : https://github.com/webispy/checkpatch-action Below is a draft of that proposed CONTRIBUTING file :

Code format

Code format should follow https://kernel.org/doc/html/latest/process/coding-style.html

With the following exceptions:

• The line length is 100 columns or fewer. In the documentation, longer lines for URL references are an allowed exception.

• Add braces to every if, else, do, while, for and switch body, even for single-line code blocks. Use the --ignore BRACES flag to make checkpatch stop complaining.

• Use spaces instead of tabs to align comments after declarations, as needed.

• Use C89-style single line comments, /* */. The C99-style single line comment, //, is not allowed.

• Use /** */ for doxygen comments that need to appear in the documentation.

• Avoid using binary literals (constants starting with 0b).

• Avoid using non-ASCII symbols in code, unless it significantly improves clarity, avoid emojis in any case.

Checking code conformity

checkpatch does not currently run on Windows.

Code conformity can be checked automatically using checkpatch. Checkpatch is used during CI to check code style after a PR.

To check code conformity :

• Mark the script as an executable chmod +x ./checkpatch.pl
• Launch the check using ./checkpatch.pl -f --no-tree file_to_check.c

 Code formatting

A tool called Clang-format can be used to format the code.

• install clang-format using sudo apt install clang-format
• install vscode extension clang-format from Xaver Hellauer

clang-format automatically load coding rules contained in .clang-format at the project root.

[jalinei]

A preliminary draft adding required files for checkpatch CI and config. As well as the proposed skeleton of CONTRIBUTING.md https://github.com/jalinei/Core/tree/contributing_guidelines

[guigur]

* The line length is 100 columns or fewer. In the documentation, longer lines for URL references are an allowed exception.

I'm okay with that exception 👍

* Add braces to every if, else, do, while, for and switch body, even for single-line code blocks. Use the --ignore BRACES flag to make checkpatch stop complaining.

yes 👍

* Use spaces instead of tabs to align comments after declarations, as needed.

I disagree I think tabs are better for aligning comments 👎

* Use C89-style single line comments, /*  */. The C99-style single line comment, //, is not allowed.

I don't thing we need to ban // comments but discourage their utilization

* Use /**  */ for doxygen comments that need to appear in the documentation.

👍

* Avoid using binary literals (constants starting with 0b).

Why? Sometimes it is clearer than hexadecimal

* Avoid using non-ASCII symbols in code, unless it significantly improves clarity, avoid emojis in any case.

👍

[Ayoub-Farah]

Use C89-style single line comments, / /. The C99-style single line comment, //, is not allowed.

// are useful in certain case to give a short comment on the same line, they should be kept.

[jalinei]

I suggest that we adapt the severity of these rules as they might be annoying but that we stick to Zephyr RTOS coding style inheritance. As such we'd get warnings if not using these styles but we still try to adapt to it.

Otherwise I suggest that you propose a different strategy than rules inheritance. I don't mind choosing another set of rules, just tell me which, and why this choice would be relevant.

[cfoucher-laas]

As one of the main contributors to the Core Zephyr modules, here is a bit of historical context: At the time we started this project, I made the choice to separate between end-user-oriented code, that followed a CamelCase style, and the buried code, not for use by anyone outside the OwnTech team and the very advanced users, that followed a snake_case style. However, thus may be something we should re-consider if we want to harmonize code (or maybe not).

On the points you mention:

• The line length limitation is something I never understood outside comments for readability. By myself, I always split what I can in multiple lines instead of having fewer but complicated lines of code (I trust the compiler for optimization), thus adding an additional restriction for code formatting is a thing I don't adhere to (even while I know it's in use in many projects). I won't oppose to it, but I would like to understand the why.
• Braces everywhere is something I adhere to (even if I sometimes take shortcuts for single-lines if). What about the braces position? I personally prefer opening braces on a new line for braces alignment. Personal preference only.
• Regarding spaces and tabs, I have a personal preference for tabs (that will render according to one's specific preferences) over spaces. Plus spaces take more bytes, thus more storage, thus consume a tiny bit more electricity in storage and git transactions (yes, negligible, but...). No strong opinion against spaces, just a personal preference.
• Regarding standard comments style, sometimes single-line comments are of use to my use-case. I would not oppose a standardization if pushed toward, but I do not see a clear advantage to following a "only multi-line comment" rule. Maybe it has an advantage of some sort, but I can't see of any for now.
• Nothing against a single convention for doxygen comments.
• Finally, regarding 0b and emojis, I have no knowledge of these of these in the current code base. For emojis (and non-ASCII chars) I can understand (until some point, as Git is Unicode in any case). Regarding the 0b however, this exclusion is a thing I don't understand why we should restrict, as C and C++ both allow for them, and they can be of use, e.g. for masks clarity. To be clear, I never used them in any code of mine, but I do not understand why we should ban them.