Re: Any concrete plans after the GDB BoF?

[Andrew Burgess <aburgess@redhat.com>]

Luis Machado <luis.machado@arm.com> writes:

> On 2/13/23 11:54, Andrew Burgess via Gdb wrote:
>> Mark Wielaard <mark@klomp.org> writes:
>> 
>>> Hi Andrew,
>>>
>>> On Sat, Feb 11, 2023 at 05:13:37PM +0000, Andrew Burgess wrote:
>>>> Simon Marchi via Gdb <gdb@sourceware.org> writes:
>>>>> I would suggest mandating one version, and for that version to
>>>>> continuously be the latest stable version of clang-format, like we do
>>>>> for Black.  When a new version comes out, we don't have to wonder if /
>>>>> when we move the next version.  Someone just pushes a patch re-formating
>>>>> the code to the next version, if there are some differences.  It keeps
>>>>> the overhead to a minimum.
>>>>
>>>> I dislike our policy of using the latest version of black, and would
>>>> argue that always using the latest version _increases_ the overhead,
>>>> rather than reducing it.
>>>
>>> Have you found the python formatting flagged by black "unstable"?
>> 
>> No.
>> 
>>>                                                                     The
>>> buildbot uses the latest black as comes with fedora stable and I don't
>>> remember it flagging issues on upgrades. But maybe it hasn't been
>>> running for long enough? It has been running since July last year. Are
>>> you running a much older black? Does it produce different formatting?
>> 
>> No.  And we don't have a huge volume of Python code.  Both of these
>> points (stability + small code size) is why I've never said anything.
>> That doesn't mean I think the idea of constantly chasing the latest
>> version is a good idea.
>> 
>> In fact, it probably makes it worse.  I _don't_ update black.  Why?
>> Because what I have just works.  When something does change I'll
>> certainly commit some incorrectly formatted code.
>> 
>> Does that really matter?  I don't think so.  It'll be an easy fix, it's
>> just annoying.
>> 
>
> I suppose that's the point of introducing auto-formatters. If some incorrect formatting is
> pushed alongside some code, it is not a big deal. But having to manually chase some format and fix it by hand (as we do now)
> before it can go in is potentially worse.
>
> It is also a burden for reviewing. It doesn't seem like the kind of thing people should be doing manually at
> this point in time.

I've never bought this argument.

This makes perfect sense in a corporate environment, where you can know
everyone is using the same tools.  But for a distributed project, I
don't think we can rely on every contributor using, or remembering to
use the formatting tools correctly.

Ideally we don't want every commit, or a daily commit, where someone
runs clang-format and posts the fix (obviously this will happen, but the
goal would be to keep this to a minimum, right?), so reviewers still
need to think about formatting when reviewing patches.

For larger patches, this is easy enough, I install the patch in my local
tree, run clang-format, and if any changes show up, I immediately reject
the patch.  But I _still_ have to think about formatting, I just do
different things.

For smaller patches that I might previously have reviewed in my email
client; well now I _really_ need to think about formatting, because if I
see anything that's even slightly weird, I can no longer make a
judgement call on if it's formatted reasonably, I absolutely _have_ to
install the patch and clang-format it in order to check it was formatted
correctly.

Now, where this might save time is if we had some kind of git hook which
could validate the code was formatted correctly and reject push attempts
if they are not formatted.  Then I could stop thinking about
formatting.  But until then .... I don't think reviewers will be able to
stop asking: is this formatted correctly?

Thanks,
Andrew

>
> Obviously the burden is different for different people and different setups.
>
>>>   
>>>> If I had a choice then, personally, I'd vote against using clang-format
>>>> at all, but it feels like there's a majority in favour, so if we do have
>>>> to go down this route, I'd rather we adopted the same policy as for
>>>> autotools and C++ versioning.  That is, pick something that works for
>>>> us, and commit to it over the medium term.  That way at least, I can
>>>> build a single version of clang-format and know that it's going to last
>>>> me for a while.
>>>
>>> But is there already a verions that works? I think that is the
>>> difference between the python black formatter for python code and the
>>> clang-format for C and C++ code. It seems for the python code there is
>>> a supported format that matches what is used, but for clang-format
>>> there is not (yet?).
>> 
>> I'm a little confused by your point here.  You (correctly) point out
>> above that the output from black is pretty stable across versions.
>> 
>> But here it almost seems like you're suggesting that we should chase the
>> latest clang-format because it doesn't (currently) support the style we
>> use.  Which would seem to suggest we are hoping it _will_ change, which
>> suggests output instability, which, surely, is a bad thing?  But like I
>> said, I didn't really understand the question here...
>> 
>> I would suggest that if we did start using clang-format, then that
>> indicates we are happy enough with its output.  If we're happy enough
>> with its output today then I think we can be happy with the output for 1
>> (or even 2) years before looking to see if an updated version offers
>> improved formatting.
>> 
>> Remember, there are folk who maintain out of tree forks of GDB.  And
>> though we shouldn't make policy choices just to accommodate them, I'd
>> hate for us to go out of our way to make their lives harder just for the
>> sake of chasing the latest version of some tool.
>
> I think Mark's point is just that we haven't settled on a particular gnu-for-clang-format rule set.
>
> Yes, there is a gnu style there already, but we haven't decide if it is good enough or not.
>
> We just need to play with it for a bit and see if people overall think it is good enough.

OK.

Thanks,
Andrew

>
>> 
>> Thanks,
>> Andrew
>>