Re: [Qemu-devel] [PATCH] net: Add SunGEM device emulation as found on Apple UniNorth

[Alex Bennée]

Benjamin Herrenschmidt <address@hidden> writes:

> On Sun, 2016-08-14 at 15:54 -0700, address@hidden
> 1.amazonaws.com wrote:
>> Hi,
>>
>> Your series seems to have some coding style problems. See output
>> below for more information:
>
> Bla bla bla bla ...
>
> Seriously, we have nazi stylebot on the list now ?

Patchew does more than just run checkpatch, it also builds and runs on a
number of different distros/build environments. In this case however
it is automatically running a check that saves reviewers time from
having to do it themselves. I'm certainly in favour of having tools on
the list that reduce the reviewers burden.

> I made conscious decisions to style it this way and as far as the tabs
> in the .h are concerned, that's because the file is lifted from Linux
> and I'd like to keep it intact as much as possible.

There are certainly sometimes where you can argue it is worth violating
checkpatch for the sake of consistency in the file however in those
cases you should:

  - mention it in the commit message
  - ignore patchew's email unless human reviewers take issue with the violation

However:

>> total: 428 errors, 73 warnings, 1950 lines checked
>>
>> Your patch has style problems, please review.If any of these errors
>> are false positives report them to the maintainer, see
>> CHECKPATCH in MAINTAINERS.

It seems very unlikely all of the reported errors here can be justified
along the lines of internal consistency, especially for adding a new
file.

--
Alex Bennée