Re: [PATCH v2 5/8] clang-format: avoid braces on simple single-statement bodies

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux