Coding style & formatting

[ladislas]

In my experience on working and contributing on a bunch of open source projects, I usually find that I spend a lot of time thinking and matching the coding style and formatting of the current code.

Teams and projects who define, share and enforce those coding styles and formatting make it very easy for contributors to follow them.

It's of course not just about listing the rules, it's helping people setting up tools to automatically implement them so that they don't have to thing about it.

Of course it's important to have CI also check those, but it's a real pain in the neck to discover all you've done wrong in CI and to need to reformat all the code and fixup all the commits to avoid the Formatting everything commit in the PR.

For our https://github.com/leka/LekaOS project, we use different tools and configs to do that:

EditorConfig

• https://editorconfig.org/

This is a small configuration file that tells the IDE you're using how to handle formatting of general files, like space vs tabs, indentation size, new line at the end of file, etc.

Here is an example configuration: https://github.com/leka/LekaOS/blob/develop/.editorconfig

It works with all the IDEs, so there's not reasons to not use it.

💡 IMHO, this is a mandatory thing to add

astyle or clang-format

Automatic formatting is very important and should even be made a pre-commit hook.

mbed-os uses astyle (https://astyle.sourceforge.net/) which I've never used before but seem to work great.

For LekaOS we use clang-format, which we really love. It made us discover the whole clang suite of tools like clang-tidy which are amazing to use to write better code.

Both tools integrate very well with vscode to do automatic formatting.

• https://github.com/welkineins/vscode-astyle
• https://github.com/xaverh/vscode-clang-format

💡 IMHO, for simplicity we should keep using astyle but update the documentation to ask contributors to install the extension first before making modifications

For people using both, I'm not sure how to tell vscode which one to use, I'll look into that.

CMake extensions

Using the CMake Language Support extension (https://marketplace.visualstudio.com/items?itemName=josetr.cmake-language-support-vscode) adds auto formatting to list files in vscode.

The default formatting is pretty fine and we haven't done any customization for LekaOS.

As mbed-ce adds and modifies a lot of list files, it's important to implement auto formatting.

The extension mentioned above does a great job, but it would be best to add formatting control to CI as well.

In this case, cmake-format takes is a step further https://github.com/cheshirekow/cmake_format

Besides very interesting and customizable formatting, it's also possible to add ou custom commands to make sure they are formatted the right way.

A vscode extension is also available https://github.com/cheshirekow/cmake_format

⚠️ That being said, the cli and extension have not been updated in a long time -- not an issue if it works out of the box

💡 IMHO, it would be nice to move to cmake-format to use the default rules and/or define our owns

General Style guide

To avoid the patchwork effect, as we develop the project more rules can be written down. A good rule is a rule that can be enforced automagically, both locally on the contributor's computer, and in CI.

This makes it easy for contributors to contribute without thinking about the rules too much and remove the burden for the reviewers to spot all the places where the rules are not followed.

In extension to that, going for automatic default formatting, even if it's not the nicest to watch, is always better than manual custom formatting.

What do you guys think?

cc @multiplemonomials @JohnK1987 @JojoS62 @wwatson4506 @jay-sridharan

[multiplemonomials]

Thanks for this detailed writeup! I have to say, as a developer, my preference tends to be towards not wanting to spend time on formatting stuff; I honestly don't care too much if different parts of the codebase are formatted differently. But I also understand that it's important to make the code readable and consistent. So, I'd be happy to accept PRs that make these changes -- adding an editorconfig sounds like a good start.

One thing I will say is, I've had bad experiences in the past with cmake-format. Not only can it not match the CMake indenting formats commonly used on Mbed, but it also has a nasty habit of producing different results when run on Windows and Linux. I actually tried to make a PR several years ago to add it to mbed-os, but gave up because of this. So I'm not sure if there's any good solution available for formatting CMake code.

[ladislas]

I'll start making PRs then :)

Regarding cmake-format, the alternative is to install CMake Language Support extension and to use their formatting when editing a file. There would be no CI, but we can live with that.

I'll make a PR showing what impact it can have to let you decide

[multiplemonomials]

What about CLion users though? It isn't really acceptable if the only way to perform autoformatting is through a VS Code-only (Or CLion-only) plugin I'm afraid.

[ladislas]

How many users are we talking about?

VSCode is free. CLion is not free for organizations and professional use.

We could apply as an open source projet to have a free license.

Also, we are not asking users who want to develop a product with mbed to use vscode only. It's for developers who want to contribute in a meaningful way to mbed-ce.

It does not mean you cannot use vi or nano if you want to. But it still means you have to follow the rules.

And when talking about developers who want to contribute in a meaningful way to mbed-ce, I think we are it.

So it's up to us to decide.

[multiplemonomials]

That's fair I guess. I just hate having to be in a situation where I have to open up a completely different IDE to autoformat files just before committing every time. There was one project at my job that had a setup like that (for CSVs, weirdly enough) and it was awful.

[JojoS62]

there is also a mix of camel case (which was used in Mbed API before) and snake case which seems to be standard now. Was there some decision to change the style? There is also a .astylerc format description in the mbed root dir.