Re: automated coding style tool

Andrew Burgess via Gdb
2025-01-17T13:44:00Z

Luis Machado via Gdb <gdb@sourceware.org> writes:

> On 1/14/25 23:04, Tom Tromey wrote:
>> Simon> If we don't like what the tool outputs, we can argue over than
>> Simon> and change the tool.
>>
>> Yeah - but that's where we're already at. Like, I ran clang-format on
>> gdb and read the diffs and found a bunch of things I didn't like. IIRC
>> the main offenders were bin-packing. I imagine Pedro did this too,
>> since he's made similar comments in the past.
>>
>> I at least CCd myself on upstream bugs against the tool. I commented
>> on some, maybe filed some too (don't remember).
>>
>> I'm not super interested in hacking on clang-format, but the tool is
>> there and all the information for someone who is.
>>
>> Simon> Also, if are missing some features to get the output we want, nobody is
>> Simon> going to magically implement them for us. And if we don't use the tool,
>> Simon> there's no motivation for us either to go implement the changes. I
>> Simon> think that the only way to get the ball rolling is to start using the
>> Simon> tool, even if the output is not ideal, and then if there's something
>> Simon> really annoying, one of us *might* have the motivation to go improve the
>> Simon> tool.
>>
>> I'm in favor of using a tool but my view is that it has to meet some
>> minimal standard of usefulness. I just think clang-format does not do
>> this.
>
> That's fair. But are we factoring in the amount of time spent over the years
> telling contributors "you forgot a space here", "two spaces after period"?

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.

And the "two spaces after a period" rule is for comments, so that's not
going to change (I hope). I mean, it's not that I care about 2 spaces
vs 1, but we need _a_ rule so that we have consistency.

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.

> I tend to think that is not a good use of one's time. I reviewed some output
> from applying clang-format --style=gnu on gdb's sources, and it looks generally
> fine to me.
>
> Some stuff is a bit different and some other stuff is very different. But
> personally I'm willing to see this automation go through and I'm fine
> adapting to a potentially new format, whatever that is.

Like you, I don't particularly care what the preferred style is, just so
long as we pick something and stick to it. I just don't think using a
particular tool is going to be the win everyone seems to think it is.

Just my thoughts.

Thanks,
Andrew