Hello, Jan!
On 24.02.25 12:55, Jan Beulich wrote:
On 23.02.2025 08:42, Oleksandr Andrushchenko wrote:
On 20.02.25 03:34, Stefano Stabellini wrote:
On Wed, 19 Feb 2025, Oleksandr Andrushchenko wrote:
Yes, I do agree. But only if we talk about having an automated
code style check now (which is definitely the goal at some time).
Before that we could still use the tool to take all that good that
it does and manually prepare a set of patches to fix those
code style issues which we like.
Let's explore this option a bit more. Let's say that we write down our
coding style in plain English by enhancing CODING_STYLE. This newer
CODING_STYLE has also a corresponding clang-format configuration.
Now, we cannot use clang-format to reformat everything because, as we
are discovering with this example, clang-format is overzealous and
changes too many things. Instead, we take "inspiration" from
clang-format's output but we manually prepare a patch series to apply
code style changes to Xen as the coding style today is not uniform
across the code base and also not always conforming to CODING_STYLE.
At this point, we have already made an improvement to CODING_STYLEe, and
made the Xen coding style more uniform across the codebase.
But do we also have an automated coding style checker that we can use?
It really depends on what that coding style would look like and
if the rules we impose are supported by clang-format and if we ready
to change ourselves if they are not.
Again, here we are trying to do what we already did, but in the opposite
direction: "diff -p" didn't work as expected(?) and we have changed
*our* coding style to *fit that tool*. So, are we ready to do the same for
another?
With a small but relevant difference: "diff" is a tool that people have been
using forever.
This is true. It is also true that that respectful tool expects labels to
use no indentation at the function scope.
Can we use clang-format to check new patches coming in?
Yes, we can use git-clang-format for that. clang-format is flexible enough
in its configuration. So, it can be used for checking patches with different
coding styles by providing .clang-format files at any folder level, e.g. we may
have something like (just to show an example):
- xen/drivers: Linux style .clang-format
- xen (rest): Xen style .clang-format
- libxl: its own .clang-format
- etc.
We can also disable formatting for the entire folder if need be by putting
a .clang-format with "DisableFormat: true" option in it.
clang-format respects the nested configuration files
Folder granularity is likely far too coarse.
That was just an example of what we can do.
So, to answer your question: I think we can use the tool to check incoming
patches.
I think the question was more aiming at: Can we have the tool check a patch
for it to only introduce well-formed code, even if elsewhere in a file being
touched there are instances of what the tool would re-format?
I need to do some tests, but with git-clang-format and possible some
pre-commit hook we can have formatting for the patch only.
Jan
Thank you,
Oleksandr
