[libvirt PATCH 0/1] DO NOT MERGE: RFC: targetted usage of clang-format

[Martin Kletzander]

On Tue, Nov 15, 2022 at 10:59:09AM +0000, Daniel P. Berrangé wrote:
>While we have a code style, it is not perfectly applied across the code
>base because its impossible for humans to manage that without using
>automated tooling. clang-format is the closest we'll get to a code
>formatter we could use, but still it would reformat quite alot of our
>code.
>
>I discovered that '/* clang-format off */' can be used to stop it from
>reformatting sections of code. It is not practical to add that comment
>around all places we don't want touched. I thought we could perhaps
>use it as a way to limit clang-format to merely do sorting & regrouping
>of #include statements though.
>
>This change illustrates what that would look like for the src/util
>directory, so we can consider whuether it is worth it.
>
>I've included a mark.pl script that I usd to auto-add the magic
>comment. It gets it wrong sometimes, so needs inspection. If we
>did decide to use this, we would need the magic comment in every
>existing source file.
>
>Then there is the question about new source files. If a contributor
>forgets to add the comment, then entire new source file will be
>processed by clang-format. This might be desirable. If so, we will
>need to fully expand the .clang-format config file to match ou4r
>desired style. Right now I only recorded include file rules.
>
>Or we could just wrong a dumb script to do #include sorting
>ourselves and carry on ignoring clang-format.
>
>I'm pretty undecided myself.
>

Few of my personal comments below, beware that they are very subjective.
They relate to different paragraphs above, so I just dumped them
altogether.

Lately I am becoming more and more appreciative of code formatters.  The
consistency is certainly appealing when reading and understanding the
.code flow.

However, unless there are no false positives we cannot depend on it 100%
of the time (e.g. in CI or other checks) and it stops being used by
contributors and reviewers.  So picking one thing that it does might be
okay as a starting point, but having all the extra stuff in the source
feels messy.  Especially when not all files are covered the same way.

And I do not think a tool needs to completely adapt to our style, just
like I do not blindly expect others to follow my style.  Consistency
brings more to the table than each person trying to push for their
preferred way of formatting.  It can be beneficial for the people to
adjust to the tool.  Well, in a reasonable manner of course.

I would much rather prefer adjusting to a tool that covers all parts
of the formatting than using just a part of what a tool does and adjust
the code and tool configuration for just one (unimportant) thing.

Sorted includes are not something I feel is helpful when reviewing the
code, for example.  It could be much cleaner if we, for example, had a
global util.h and conf.h which would include util/*.h and conf/*.h,
respectively, in a correct order, as that would make it a bit more
error-proof, even though I don't consider that particularly nice either.

So I, personally, do not like messing with the code, adding duplicated
comments and scripts.  Instead I am willing to sacrifice some of the
things we are used to, decide on a configuration file for a formatter
that covers every piece of indentation and formatting (e.g. does not
skip some parts of the code because it does not understand them, even
though there would be changes it might take some time getting used to),
and then ideally then toss all related syntax-checks, but have
consistent code format that we can check for in CI and locally.  Not to
mention that most editors, especially when combined with an LSP, already
support using the formatters when saving a file, for example.

Anyway, those are my €.02, thanks for reading the whole *subjective* essay =)

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20221115/501025d0/attachment.sig>