To fail or not to fail. That is the question! :) I'd add to that the contributor documentation should be updated to add steps for having a clang-format (or any other formatter) as a git pre-hook.
I'm partial to failing the build (in CI) to avoid using further electricity if the first step (syntax) fails, as another commit fixing the syntax will likely come after. ~Nico On 17 Dec, Илья Шипицин wrote: > Well, there are formatters like clang format or similar. We can even run them > in pipeline. But it has nothing to do with compilation and we should not fail > build because of that > > On Thu, Dec 17, 2020, 5:35 PM Nicolas CARPi <nicola...@rpi.ooo> wrote: > > Dear list, > > Formatting on PRs is a hot topic, but IMHO it should be done by an > automated tool so we can forget about it and focus on the core of the > PR, not the style. A git pre-hook would be best. > > > I'd say it is friendly mood not to push people to comply formatting (who > cares > > of indentation ?) > I disagree with that view. No need to be as hardcore as go who won't > compile if there is an indentation issue, but still, I think it helps > readability and prevent bugs to have correct indentation (an incorrect > one might make you think that a block is part of a loop, when it is > not). > > Regards, > ~Nicolas CARPi > > PS: go ships with a formatting tool so this is a non issue in that > language > > > On 17 Dec, Илья Шипицин wrote: > > well, I met some projects that are insane on formatting. > > it is funny each time, when discussing new PR "please fix your > formatting" > > (weeks spent on that), after PR is merged something is broken because of > lack > > of testing. > > > > I'd say it is friendly mood not to push people to comply formatting (who > cares > > of indentation ?) > > > > чт, 17 дек. 2020 г. в 13:05, Tim Düsterhus <[1][1]t...@bastelstu.be>: > > > > Ilya, > > > > Am 17.12.20 um 08:26 schrieb Илья Шипицин: > > > gcc 11 (delivered with fedora rawhide) introduces indentation > check. > > > it is totally harmless. > > > > > > this patch suppresses it. > > > it resolves #998 > > > might be backported to all supported branches. > > > > > > > Can we please fix the actual issue instead of suppressing the > warning? > > In fact I believe that this line is incorrectly intended: > > > > [2][2]https://github.com/haproxy/haproxy/blob/ > > a4009cd6103a92752db27c3a85051c6adcc832c1/include/import/plock.h#L236 > > > > Best regards > > Tim Düsterhus > > > > > > References: > > > > [1] mailto:[3]t...@bastelstu.be > > [2] [4]https://github.com/haproxy/haproxy/blob/ > a4009cd6103a92752db27c3a85051c6adcc832c1/include/import/plock.h#L236 > > -- > ~Nico > > > References: > > [1] mailto:t...@bastelstu.be > [2] https://github.com/haproxy/haproxy/blob/ > [3] mailto:t...@bastelstu.be > [4] > https://github.com/haproxy/haproxy/blob/a4009cd6103a92752db27c3a85051c6adcc832c1/include/import/plock.h#L236 -- ~Nico
