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
Attachment:
signature.asc
Description: PGP signature
