Phillip Wood <phillip.wood123@xxxxxxxxx> writes: > Hi Karthik > > On 08/07/2024 21:25, Karthik Nayak wrote: >> Junio C Hamano <gitster@xxxxxxxxx> writes: >> >>> Karthik Nayak <karthik.188@xxxxxxxxx> writes: >>> >>>> Set the 'RemoveBracesLLVM' to 'true' which ensures that we avoid curly >>>> braces for single-statement bodies in conditional blocks. >>> >>> Hmph, two warnings in its documentation [*] sound ominous, especially >>> the second one that says: >>> >>> Warning >>> >>> Setting this option to true could lead to incorrect code >>> formatting due to clang-format’s lack of complete semantic >>> information. As such, extra care should be taken to review code >>> changes made by this option. >>> >>> which implies it may not necessarily a good idea to add to >>> automation without telling contributors that they may get hit with a >>> false positive (or incorrect rewrite). >> >> Agreed on this one. I'm a bit skeptical to be honest too. I think I >> should have added information about the warning in the commit too. I >> will for next round. Overall, this also contributes to the reason why I >> decided these CI jobs need to be allowed to fail. > > I'm a bit confused - what does "allowed to fail" mean? We don't have any > automated requirements on the CI passing. We don't want to train > contributors to routinely ignore CI failures but if they don't get a > notification that this job failed how do they know to correct the > formatting? > Well, it mostly means that the CI would show the style-check job as failed, but the overall pipeline would be successful. We want to ideally fail the pipeline too, but I'm being careful to not disrupt things suddenly and I think once we see what false positive rate is and once we fine tune this settings maybe over the next release or so, we can enforce this. [snip]
Attachment:
signature.asc
Description: PGP signature
