Skip to content
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

Setup and use clang-format as part of CI\CD #280

Open
NikolajSchlej opened this issue Aug 28, 2022 · 6 comments
Open

Setup and use clang-format as part of CI\CD #280

NikolajSchlej opened this issue Aug 28, 2022 · 6 comments

Comments

@NikolajSchlej
Copy link
Collaborator

Manual code formatting is hard and there are almost always some inconsistencies and preferences, which are sometimes really hard to resolve. We are in luck though, because sometimes the resolution is simple: make a robot do formatting for you, then make that robot do it every time for every code change.

Result: everybody are about as equally pissed off, but at least the code looks consistent. Let's do it for this repo too.

@vulpes2
Copy link
Contributor

vulpes2 commented Jan 30, 2023

I can take a look at this, but I think the best way to implement this is in a pre-commit hook in git, instead of giving a workflow write access to the repo. It would be better if someone who is more familiar with the codebase can come up with the clang-format config though, since I don't know the codebase very well.

Here's what I came up with and it's probably not good enough, it still makes a lot of changes that sacrifices readability for correctness:

BasedOnStyle: LLVM
IndentWidth: 4
UseTab: Never
DerivePointerAlignment: True
PointerAlignment: Left
BreakBeforeBraces: Stroustrup
ColumnLimit: 120
IndentCaseLabels: True

@NikolajSchlej
Copy link
Collaborator Author

Looks good as the first iteration input, but needs a exclusion rule for 3rd-party code, if such things are possible, otherwise we'll reformat stuff that is not ours.

@NikolajSchlej
Copy link
Collaborator Author

I'm fine with something like make uncrustify that will do the step manually for now, until all of us contributors are content with the results. Pre-commit hook would be good too later on.

@vulpes2
Copy link
Contributor

vulpes2 commented Jan 31, 2023

Looks like it is not possible to exclude individual files with clang-format, but excluding directories is possible. Which directories contain 3rd party stuff that we don't want to lint?

@NikolajSchlej
Copy link
Collaborator Author

Every subdirectory of common/.

@vulpes2
Copy link
Contributor

vulpes2 commented Jan 31, 2023

That makes it easy, we just need to put some more config files in these directories to disable formatting. The config from the subdirectories have higher priority:

DisableFormat: true
SortIncludes: Never

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants