All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, famz@redhat.com
Subject: Re: [Qemu-devel] [PATCH] net: Add SunGEM device emulation as found on Apple UniNorth
Date: Wed, 21 Sep 2016 11:16:45 +0100	[thread overview]
Message-ID: <87intpblrm.fsf@linaro.org> (raw)
In-Reply-To: <1471219193.12231.78.camel@kernel.crashing.org>


Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Sun, 2016-08-14 at 15:54 -0700, no-reply@ec2-52-6-146-230.compute-
> 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

  parent reply	other threads:[~2016-09-21 10:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-14 22:48 [Qemu-devel] [PATCH] net: Add SunGEM device emulation as found on Apple UniNorth Benjamin Herrenschmidt
2016-08-14 22:54 ` no-reply
2016-08-14 23:59   ` Benjamin Herrenschmidt
2016-08-15  0:11     ` Benjamin Herrenschmidt
2016-09-21  3:18       ` [Qemu-devel] [Qemu-ppc] " David Gibson
2016-09-21  9:02         ` Benjamin Herrenschmidt
2016-09-21 10:16     ` Alex Bennée [this message]
2016-09-21 10:40       ` [Qemu-devel] " Benjamin Herrenschmidt
2016-09-21 11:52         ` Alex Bennée

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87intpblrm.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=benh@kernel.crashing.org \
    --cc=famz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.