Re: Any concrete plans after the GDB BoF?

On 2/13/23 15:13, Andrew Burgess wrote:
> Luis Machado <[email protected]> writes:
> 
>> On 2/13/23 11:54, Andrew Burgess via Gdb wrote:
>>> Mark Wielaard <[email protected]> writes:
>>>
>>>> Hi Andrew,
>>>>
>>>> On Sat, Feb 11, 2023 at 05:13:37PM +0000, Andrew Burgess wrote:
>>>>> Simon Marchi via Gdb <[email protected]> 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?
> 

That's what I have in mind. Some pre-commit hook that checks/does things. Obviously we're not there yet, but that would be the most convenient way.

I think anything that needs to be checked by hand wouldn't be an improvement compared to the current process.

> 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
>>>
>