fprime
fprime copied to clipboard
nasa
Format C++ codebase
| F´ Version | devel |
| Affected Component | N/A |
Feature Description
We are going to start formatting the entire C++ codebase and enforce the formatting through a CI check.
Plan
- Create CI workflow to check that C++ is formatted correctly, with an optional exclude list. Start by excluding everything.
- Iteratively format the C++ codebase, removing the exclusion parameter as we go.
Subtasks
- [ ] Create clang-format check workflow
- [ ] Format each top-level folder within F´, to be broken down in other issues
@JohanBertrand if you don't mind I'll take the CI part of this ticket, because we have an idea of how we want to do it. I'm working on this right now, I'll tag you in the PR if you'd like to review it
Once the check is there, feel free to help out formatting the code base!
We will format by small chunks and activate the CI per-module as we go.
One issue that is arising, and is loosely related to https://github.com/nasa/fprime-tools/pull/199
clang-format, for some obscure reason, has some slight variations in the formatting it outputs across different versions of the tool (even when using the exact same style). I have seen it happen with indentation of comments, and spacing like this one: https://github.com/thomas-bc/fprime/commit/a4585cde58301c0e201e797ea7a3ad9a7a090125
Some conversations about it here and here
We are likely going to need to pick a version of the tool we want to use in CI, version that users will need to use if they need to pass CI. It is very inconvenient... but there doesn't seem to be a way around it.
Let me know if you can think of anything else
cc @LeStarch @JohanBertrand
I think it's reasonable to fix the version of the formatter, like it is also done with the static analyser (clang-tidy-12). Worst case scenario, the minor differences between two formatted versions would be flagged by the CI, and it could be resolved in one single commit.
It might be good to select a version of clang-tidy which is the same as clang-format to make it easier for the developers.
I'm not sure which version to select however. clang-tidy-18 seem to have some new warnings and is detecting two new issues in the current configuration on the development branch. clang-format-14 is adding the absolute path for the style that we discussed in https://github.com/nasa/fprime-tools/pull/199, which might be great to keep. The space difference that you mentioned as an example was introduced between clang-format-17 and clang-format-18.
Clang tidy/format 12 to 18 are easily available on ubuntu20.04 and 22.04 (I'm not sure about Mac OS), so any of those version would work for me.
For fprime-util format, we will need to decide if we want it to fail when the version is different than the one we expect. I guess we could send a warning during the formatting but still process the files if the version is different. What is your opinion?