qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] Restore consistent formatting


From: Blue Swirl
Subject: Re: [Qemu-devel] Restore consistent formatting
Date: Sat, 11 Feb 2012 09:19:14 +0000

On Wed, Feb 8, 2012 at 15:36, Andreas Färber <address@hidden> wrote:
> Am 08.02.2012 16:23, schrieb Anthony Liguori:
>> On 02/08/2012 09:04 AM, malc wrote:
>>> On Wed, 8 Feb 2012, Andreas F?rber wrote:
>>>
>>>> Arbitrarily reformatting your files is not okay. If you want a different
>>>> formatting, you need to fix checkpatch.pl first to not error on that
>>>> formatting in your files.
>>>
>>> It was always formatter like this (internally consistent), then others
>>> added code which made it not so.
>>
>> We do have a mixed style in the audio layer.  I'm not happy about that
>> but I also feel strongly that going through and doing a reformat is not
>> a worthwhile exercise.
>>
>> I can also understand the desire to keep things consistent.  But patches
>> should always go to the mailing list.  I certainly would have acked such
>> a patch FWIW.
>>
>> I think people get a bit too excited about coding style.  There are much
>> more important things to worry about in life than the number of spaces
>> before a parenthesis :-)
>
> This is not about whether or not we put a space somewhere.
>
> It's about reviewers and SubmitAPatch telling people to run
> checkpatch.pl on patches and checkpatch.pl reporting this as an ERROR,
> not a WARNING. So if you follow Stefan's instructions on running the
> script as a commit hook (which is the only sane way to run it when
> handling lots of patches) you can't commit a patch or your local changes
> when there are ERRORs.

The warning levels are from Linux, they could well be incorrect.

> I just spent half the night trying to find out why checkpatch.pl reports
> CPUX86State *env, CPUYState *env, CPyState *env as ERRORs but not
> CPUState *env. I did not succeed in really understanding it.

Maybe the CPUState #defines in target-*/cpu.h confuse checkpatch.

> So either we need to all stop using and telling to use checkpatch.pl or
> someone needs to fix it.

It's not that black or white. Obviously checkpatch.pl can make
mistakes and some of its rules are actually not based on CODING_STYLE
but they are in line with the current code (except for example in this
audio case), so the output is usually helpful to make code more
uniform. I wouldn't "fix" checkpatch to consider local style
variations. If we don't want a rule that complains spaces after
function identifiers, it could be removed entirely. Though then
someone could submit patches to introduce them elsewhere, but if there
is no rule, that should be OK.

> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



reply via email to

[Prev in Thread] Current Thread [Next in Thread]