Re: automated coding style tool

Joel Brobecker via Gdb
2025-01-17T15:16:08Z

> I don't think adding this tool is going to be the magic bullet everyone
> seems to think it is.
>
> As a reviewer you'll either still need to learn the style in order to
> spot when a contributor has not running the formatting tool. Either
> that, or you need to apply each patch locally and the check run the
> formatting tool to check the formatting is correct.

I have experience with a project where developers were asked to install
a pre-commit check that would run the formatting tool, but where we did
not have (yet) a pipeline to verify the formatting at submission time.
This project had a lot of developers, some of them very occasional.

It's true that, at first, we had a number developers forgot to install
that pre-commit hook, and ended up submitting ill formatted code.

What I noticed, at least for myself, is that with time you start
developing an eye for what the expected formatting is like. And when
I noticed something odd, I'd just ask if they installed the pre-commit
and ran the formatting tool. If they say no, I pointed them to
the directions, and asked them to retest and resubmit. Ultimately,
we mostly converged.

And for the cases we didn't notice at review - no big issue.
The formatting shouldn't be horrible of the reviewer would have
noticed it, so no great harm -- certainly no worse than right now.
And later on, the next developer touching the same file ends up
noticing that the formatting seems wrong. It's a bit of a pain
for them to first run the formatting tool and push a pure-reformatting
commit. But we tell the contributor who forgot, they install the tool,
and normally we eventually converge.

Another things we can imagine doing is a nightly job on sourceware
that runs the nightly snapshot through the formatting tool, and
sends an email with a list of files that are not conformant.

Where I think it might be painful is which version of the tool
to use. Different versions will likely have slightly different
results in terms of formatting the code. You might want to require
a specific version and check for that version, to avoid different
users having different ways of formatting the same code.

> I think if GDB could just move away from the mailing list and each users
> pushes their own patches model, over to a pull request style approach,
> then we could potentially have _real_ automated checks, e.g. checking
> the formatting. Then we have an _actual_ win.

Seconded.

In a model like this, you don't have to ask. You run a job that
checks it, and you know if it's conformant or not.

--
Joel