[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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |