Junio C Hamano <gitster@xxxxxxxxx> writes: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> 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. >> >> The latter seems to be of concern. But since we only use clang-format to >> verify the format and not to apply formatting, we should be okay here. > > Hmph. Could you tell me where I can read more about "we tell > clang-format only to verify but not to apply"? If that is truely > the case, perhaps I shouldn't be worried to much, but it is not > clear to me how we enforce that this is to be used only for > verification with non-zero false positive, and never for > reformatting before submission. > I was referring to the fact that, we expose '.clang-format' via 'make style' which only prints the diff to the STDOUT. The user has to still manually make these changes. However users could be using tools to auto-format on save and this could be an issue. > The senario I was worried about was this. We aadd to .clang-format > that is in-tree, and not just CI jobs but our human contributors may > use it to check what they newly wrote before committing and they may > even take the differences as suggested fixes (which may end up > breaking their working code). > > Thanks. I totally see your point here. If the contributors do end up with bad formatting because clang messed it up, that is an okay situation, since that shouldn't happen often, and when it does, it would be the same situation as without this check, wherein we rely on reviewers. The issue is whether it would break their code, I couldn't find anything on this. But overall, while I personally find this check useful, I'm happy to drop it, My goal is to ensure we run this on CI as a first step :)
Attachment:
signature.asc
Description: PGP signature
