Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sat, Aug 21, 2010 at 12:24 PM, Markus Armbruster <address@hidden> wrote:
> Blue Swirl <address@hidden> writes:
>
>> On Sat, Aug 21, 2010 at 9:54 AM, Markus Armbruster <address@hidden> wrote:
>>> Blue Swirl <address@hidden> writes:
>>>
>>>> On Fri, Aug 20, 2010 at 6:44 PM, Blue Swirl <address@hidden> wrote:
>>>>> On Fri, Aug 20, 2010 at 1:47 PM, Markus Armbruster <address@hidden> wrote:
>>>>>> Anthony Liguori <address@hidden> writes:
>>>>>>> To be perfectly honest, we have enough hard problems to solve in QEMU.
>>>>>>> We're spending a lot more time on coding style than we probably need
>>>>>>> to :-)
>>>>>>
>>>>>> In my not so humble opinion, that's because the current CODING_STYLE is
>>>>>> idiosyncratic, widely disliked (follows from idiosyncratic pretty much
>>>>>> inevitably), widely violated by existing code, and only haphazardly
>>>>>> enforced for new code.
>>>>>
>>>>> I think Coccinelle could help us here, it can check for some of the
>>>>> CODING_STYLE issues. We only need to include it to our build system
>>>>> and add git hooks if possible. It can also perform mechanical
>>>>> conversions (if desired).
>>>>
>>>> This Coccinelle script seems to do the job:
>>> [...]
>>>> There are some formatting issues though, I get changes like:
>>>> -    for (i=0; i<5; i++)
>>>> +    for(i = 0;i < 5;i++) {
>>>>
>>>> Reformatting the expressions with more spaces is nice, but removing
>>>> the spaces between 'for' and '(' and especially after ';' is not.
>>>
>>> Please make sure that patch submitters can easily check their patches
>>> with your tool.  Depending on coccinelle isn't a problem for me, but it
>>> may well be for others.
>>
>> Coccinelle depends on OCaml and lots of other stuff, but just I
>> installed it with 'aptitude install coccinelle'. These days you can
>> also check Linux kernel sources with the included Coccinelle scripts.
>
> Could be "fun" for developers using Windows.  If they exist.

At least OCaml site offers binary download for Windows. I didn't
compile Coccinelle myself, so I don't know how much that helps.

>> But depending on Coccinelle for the build wouldn't be nice.
>>
>>> Unless you mass-convert existing code to your style, tools working on
>>> source files won't cut it, because reports of the patch's style
>>> violations are prone to drown in a sea of reports of preexisting style
>>> violations.  There's a reason why Linux's scrtips/checkpatch.pl works on
>>> patch files.
>>
>> Mass conversion would have the benefit that submitters, who use old
>> code as their reference, are more likely to use the correct style.
>>
>>> Even a working patch checking tool can only address the last issue
>>> (haphazard enforcement), not the other ones.  You may not care.
>>
>> Which other ones?
>
> Quoting myself:
>
>    [...]                                       the current CODING_STYLE is
>    idiosyncratic,

Personal preference. I liked Fabrice's style but I also like current
style. I would probably like Linux style except for the LISPisms. I
don't like GNU or Java style.

> widely disliked (follows from idiosyncratic pretty much
>    inevitably),

How widely? How do you know that?

> widely violated by existing code,

The mechanical conversion would address that. We could of course
convert to some other style, or declare that we don't care about the
style after all. Or claim that we care, don't fix old code and whine
randomly at some submitters.

> and only haphazardly
>    enforced for new code.

I'd like to fix that.

>>> I still think inventing yet another idiosyncratic coding style plus
>>> tools to enforce it is a waste of time.
>>
>> There are historical reasons for the style used in the current code
>> base. There are also reasons why CODING_STYLE was written like it
>> stands now.
>
> While wasting time for historical reasons is certainly better than
> wasting time for the heck of it, it's arguably worse than stopping the
> waste.

But how would you do that? Drop the CODING_STYLE (and accept
anything)? Switch to a new CODING_STYLE that is widely appreciated and
so all bikeshedding will cease? Enforce current style?