[Development] clang-format

Kari Oikarinen kari.oikarinen at qt.io
Wed Jun 20 09:02:43 CEST 2018



On 20.06.2018 09:30, Lars Knoll wrote:
 >
 >
 >> On 19 Jun 2018, at 18:19, Ville Voutilainen 
<ville.voutilainen at gmail.com> wrote:
 >>
 >> On 19 June 2018 at 19:13, Philippe <philwave at gmail.com> wrote:
 >>>> For the above reasons I'd lean towards not running it globally and 
just using it
 >>>> on new changes.
 >>>
 >>> +1, based on my clang-format experience on a big application.
 >>>
 >>> BTW, keep in mind that you can disable clang-format on code 
sections with:
 >>>
 >>> // clang-format off
 >>> // clang-format on
 >>
 >> When I last experienced a large-scale clang-format reformat, it really
 >> hurt development
 >> during the churn. We should somehow manage to do it during a time when
 >> there aren't
 >> many pending patches in the pipeline. I'm not concerned about
 >> git-blame; that has never
 >> been a problem after reformats. However, I do not care about
 >> indentation nor do I want
 >> to spend time on it either way, it has no actual effect on readability
 >> and maintainability
 >> of code, and consistency outside the file you're in has never mattered
 >> to me one bit.
 >>
 >> IOW, I'm not opposed to reformats and auto-checking of clang-format
 >> (or even hooking it),
 >> but I do not see it as a thing with all that great return-of-investment.
 >
 > It helps in that you do not need to point those things out in code 
reviews, and that I (and others) won’t even create changes with wrong 
formatting that I’ll need to fix up later on. It’s part of a larger 
story, where I would like to get as much automatic checking of changes 
done before humans start reviewing.
 >
 > One idea could be to introduce this incrementally. Let’s first start 
off with enforcing it for new changes. Then we run it globally over the 
code base shortly before Qt 6.0 is being released. At that time merges 
shouldn’t be as much of a problem (as we’ll probably cherry-pick into Qt 
5.15) and by then all new changes in Gerrit will be properly formatted 
(due to the earlier hook).
 >

I think this is a good approach. It includes a period of time between 
taking the
formatting options into use and applying them globally. There's a good 
chance
that the used clang-format settings may change due to discussion as more 
people
and code are exposed to it. It would be a shame to have to globally reformat
*again* after a short period of time.

(I'm still not sold on the benefit of running the reformatting on 
existing code
outweighs the cost, but the suggested time would probably give the best 
balance
there.)

--
Kari



More information about the Development mailing list