Re: [Qemu-devel] Restore consistent formatting

[Anthony Liguori]

On 02/08/2012 09:36 AM, Andreas Färber 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.

It's a suggestion, it still assumes that you are going to exercise discretion 
and make rational decisions when checkpatch does something silly.

> 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.
>
> So either we need to all stop using and telling to use checkpatch.pl or
> someone needs to fix it.

checkpatch.pl is a tool.  Tools are meant to make our lives easier, not harder. 
 You should use checkpatch.pl to help you figure out if you have coding style 
issues but it is not a QEMU maintainer that gives you a required Ack before you 
code gets accepted.  If it's doing something stupid, ignore it.

Making checkpatch 100% perfect (or event 99% perfect) is simply not worth the 
effort.  Parsing C is insanely hard and doing it in perl only makes the problem 
worse :-)

If you want to tone down the language in SubmitAPatch, please go ahead.  It's a 
wiki after all.

Regards,

Anthony Liguori

> Andreas