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

Add style handling via .editorconfig and .clang-format #203

Closed
wants to merge 4 commits into from

Conversation

evelikov
Copy link
Contributor

@evelikov evelikov commented Oct 1, 2024

Generally such topics are highly volatile, so grab as many/little as you prefer while I hide from the incoming rotten tomatoes :trollface:

This PR adds the two configuration files, alongside a CI workflow which helps getting consistent style for new changes.

The existing code is untouched - if you want to mass update the repo be my guest. If you do, adding the commit sha in .git-blame-ignore-revs will keep git blame working OOTB with Github/Gitlab, while devs will have to git config it (see git-blame(1)).

Notes:

  • the .clang-format is coming from the kernel, yet lacks any copyright attribution ☹️
  • clang-format sometimes reorders multi-line enums/initializers - a trailing comma for final entry usually solves it
  • styling can be disabled by adding clang-format off comment and re-enabled with clang-format on
  • the "80" column width is bit of guess work - no preference here
- 24 files changed, 3268 insertions(+), 3160 deletions(-) // with 80 column width
- 24 files changed, 2988 insertions(+), 3149 deletions(-) // with 90 column width

Most editors support the config format out of the box and with some
patches in the works, it's great to have something in-tree to avoid
(further) mixing the style.

The format is sufficient to handle basic indentation size/style and line
length. More elaborate style tracking can be done with clang-format...
incoming with a later patch.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Swap kernel list of for_each macros for what we actually use in-tree.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
It will help enforce the style defined in .clang-format, only for newly
introduced changes. Pre-existing style issues will not be flagged,
unless the lines are changed with given commit.

If a particular section needs to be ignored, it should be wrapped in
clang-format off/on section.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
@gregkh
Copy link
Owner

gregkh commented Oct 1, 2024

Thanks for these, no major objections but I bumped the max line length in the editorconfig file

I've merged these "by hand", to go around the build check, but all are now applied.

@gregkh gregkh closed this Oct 1, 2024
@evelikov evelikov deleted the style branch October 1, 2024 16:52
@evelikov
Copy link
Contributor Author

evelikov commented Oct 1, 2024

Thanks, glad it wasn't too controversial.

Fwiw the CI is failing due to the missing Copyright for .clang-format' ... I wonder if we can invoke your superpowers and fix that in the Linux kernel (+ copy it here) 😅

@gregkh
Copy link
Owner

gregkh commented Oct 1, 2024

All now fixed up!

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

Successfully merging this pull request may close these issues.

2 participants