[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 0/2] code style exercise: Drivers folder samples



Hello, Stefano!

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?
Funny, but I checked diffutils and they have a test case for that behavior
of the "diff -p" that we are trying to avoid. So even if we provide a patch
to diffutils we would need a good reasoning why their behavior is
wrong and needs fixing.
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

So, to answer your question: I think we can use the tool to check incoming
patches.

  Or would
clang-format flag too many things as coding style errors?
It really depends on what we decide: if we are ready to change our coding
style Or if we just want to skip entire folders from formatting, etc.

If it is flagging too many things as error, so we cannot use it as
automated checker, is it still worth going through the exercise? Yes, we
make some improvement we haven't reached the goal of having an automated
code style checker.
My impression from all these conversations is that most of the community
can see that there are lots of places where clang-format does the job
right. At the same time there are places which we do not like.
Some of those we don't like can be discussed though as some feel
like "I personally like it" or "don't like", e.g. depend on one's perception.
We would need to accept the fact that if an existing code sample does
conform at the moment, but still clang-format may re-format that code
as well. Just because it will try to improve the code. Or "improve"

So, the bottom line would be: yes, this can turn into a series of patches
which will improve the coding style and fix the obvious. And then we
can see what is left when we try to automatically run .clang-format at that
stage and decide.

We can also wait for "Year, 2034. Coding style, clang-format-AI. Attempt 21"
letter in the future.

We can also claim that "the coding style is perfect as it is, handmade and
no robots allowed".

So, I would love to hear from the community what is the best route here.

Thank you,
Oleksandr



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.