[Gluster-devel] Clang-Formatter for GlusterFS.

[Ravishankar N]

On 09/18/2018 02:02 PM, Hari Gowtham wrote:
> I see that the procedure mentioned in the coding standard document is buggy.
>
> git show --pretty="format:" --name-only | grep -v "contrib/" | egrep
> "*\.[ch]$" | xargs clang-format -i
>
> The above command edited the whole file. which is not supposed to happen.
It works fine on fedora 28 (clang version 6.0.1). I had the same problem 
you faced on fedora 26 though, presumably because of the older clang 
version.
-Ravi

>
> +1 for the readability of the code having been affected.
> On Mon, Sep 17, 2018 at 10:45 AM Amar Tumballi <atumball at redhat.com> wrote:
>>
>>
>> On Mon, Sep 17, 2018 at 10:00 AM, Ravishankar N <ravishankar at redhat.com> wrote:
>>>
>>> On 09/13/2018 03:34 PM, Niels de Vos wrote:
>>>> On Thu, Sep 13, 2018 at 02:25:22PM +0530, Ravishankar N wrote:
>>>> ...
>>>>> What rules does clang impose on function/argument wrapping and alignment? I
>>>>> somehow found the new code wrapping to be random and highly unreadable. An
>>>>> example of 'before and after' the clang format patches went in:
>>>>> https://paste.fedoraproject.org/paste/dC~aRCzYgliqucGYIzxPrQ Wondering if
>>>>> this is just me or is it some problem of spurious clang fixes.
>>>> I agree that this example looks pretty ugly. Looking at random changes
>>>> to the code where I am most active does not show this awkward
>>>> formatting.
>>>
>>> So one of my recent patches is failing smoke and clang-format is insisting [https://build.gluster.org/job/clang-format/22/console] on wrapping function arguments in an unsightly manner. Should I resend my patch with this new style of wrapping ?
>>>
>> I would say yes! We will get better, by changing options of clang-format once we get better options there. But for now, just following the option suggested by clang-format job is good IMO.
>>
>> -Amar
>>
>>> Regards,
>>> Ravi
>>>
>>>
>>>
>>>> However, I was expecting to see enforcing of the
>>>> single-line-if-statements like this (and while/for/.. loops):
>>>>
>>>>       if (need_to_do_it) {
>>>>            do_it();
>>>>       }
>>>>
>>>> instead of
>>>>
>>>>       if (need_to_do_it)
>>>>            do_it();
>>>>
>>>> At least the conversion did not take care of this. But, maybe I'm wrong
>>>> as I can not find the discussion in https://bugzilla.redhat.com/1564149
>>>> about this. Does someone remember what was decided in the end?
>>>>
>>>> Thanks,
>>>> Niels
>>>
>>
>>
>> --
>> Amar Tumballi (amarts)
>> _______________________________________________
>> Gluster-devel mailing list
>> Gluster-devel at gluster.org
>> https://lists.gluster.org/mailman/listinfo/gluster-devel
>
>