[Development] clang-format

[Frederik Gladhorn]

On onsdag 20. juni 2018 13.01.10 CEST Tor Arne Vestbø wrote:
> On 20 Jun 2018, at 12:13, Lars Knoll <lars.knoll at qt.io> wrote:
> 
> > 
> > I can’t see how clang-format will make you jump through any sort of hoops.
> > Creator already has a hook for doing it on file saving time afaik,
> > git-clang-format will clean up your change from the command line.
> 
> Good point, I was imagining it used only to verify style, not to
> auto-format. Still, starting out with a few non-controversial rules would
> be a good thing.

For now we're trying to get the format definition file into qt5 at all.
Once we have a file we agree on (and I'd be happy to see it evolve, it's in 
git, so please just contribute everyone), we can decide how to use it.

I see several things that would make sense.
1) Make it easy to use locally. That is mostly the case, just run:
   $ git clang-format
   And the lines you changed will be reformatted.

2) It's relatively easy to have a commit hook that complains or runs clang-
format for you.

3) Make it part of the sanity bot with a nice message explaining that it's 
possible to use clang-format and ask contributors to use it (with neutral or 
-1 effect).

Cheers,
Frederik
 
> Tor Arne 
> 
> 
> 
> > 
> > Lars
> > 
> > 
> >> On 20 Jun 2018, at 11:52, Tor Arne Vestbø <Tor.arne.Vestbo at qt.io> wrote:
> >> 
> >> How about we also start with only one or two  obvious rules that everyone
> >> agrees on? I don’t want Qt development to turn into Python PEP8 type
> >> rigid rules that makes you jump through a million hoops. If the latter
> >> is the goal here then I’m against enforcing clang-format, and it should
> >> be implemented as a bot that just warns, similar to the current style
> >> bot.
 
> >> - Tor Arne
> >> 
> >> 
> >>> On 20 Jun 2018, at 11:21, André Pönitz <apoenitz at t-online.de> wrote:
> >>> 
> >>> 
> >>>> On Wed, Jun 20, 2018 at 06:30:26AM +0000, 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.
> >>> 
> >>> 
> >>> It's also a cultural thing.
> >>> 
> >>> Quite a few people seem to take less offense from a "Your formatting is
> >>> bad" when the comment comes from a bot than when it comes from a human.
> >>> 
> >>> 
> >>> 
> >>>> 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).
> >>> 
> >>> 
> >>> Incrementally sounds good to me.
> >>> 
> >>> Still I am a bit of a fence here. So far I've seen a couple of auto-
> >>> formatting attempts biting back, so I thinl it would help to convince
> >>> me
> >>> to see the kind of changes that would happen first before deciding
> >>> on the global change.
> >>> 
> >>> Andre'
> >>> _______________________________________________
> >>> Development mailing list
> >>> Development at qt-project.org
> >>> http://lists.qt-project.org/mailman/listinfo/development
> > 
> > 
> 
> 
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development