From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=36170 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PsAKE-0006mO-6b
	for qemu-devel@nongnu.org; Wed, 23 Feb 2011 03:43:59 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1PsAKC-0000sO-U9
	for qemu-devel@nongnu.org; Wed, 23 Feb 2011 03:43:58 -0500
Received: from mail-ew0-f45.google.com ([209.85.215.45]:36970)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1PsAKC-0000sH-Pl
	for qemu-devel@nongnu.org; Wed, 23 Feb 2011 03:43:56 -0500
Received: by ewy24 with SMTP id 24so1908696ewy.4
	for <qemu-devel@nongnu.org>; Wed, 23 Feb 2011 00:43:55 -0800 (PST)
MIME-Version: 1.0
In-Reply-To: <alpine.LFD.2.02.1102222159010.2912@bbs.intern>
References: <alpine.LFD.2.02.1102202038360.26526@bbs.intern>
	<alpine.LFD.2.02.1102222159010.2912@bbs.intern>
Date: Wed, 23 Feb 2011 08:43:55 +0000
Message-ID: <AANLkTik4ujGrwSanLL1LJJU7q=PLwXSp1xdkEWGAVX9g@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH 2/2 V1] Fixed EPROM for AMD driver
	compatibility under DOS with Netware driver
From: Peter Maydell <peter.maydell@linaro.org>
Content-Type: text/plain; charset=UTF-8
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Gerhard Wiesinger <lists@wiesinger.com>
Cc: qemu-devel@nongnu.org

On 22 February 2011 21:00, Gerhard Wiesinger <lists@wiesinger.com> wrote:
> Hello,
>
> No comments? Can someone commit?

Not my area of the code, but some general remarks:

(1) your patch email seems to be in an odd format with two patches
concatenated, the first of which just changes a line introduced in
the first. You should fix this and resubmit as a single patch in
the right format that makes the changes you want.

(2) Style issues: QEMU's coding style is for C-style comments
(/* .. */) not C++-style //. Also lines should be 80 characters
maximum, so your end-of-line comments should probably be moved.

(3) Your commit message could be better. Convention is that the
summary (subject) should start with an indication of what area
is being patched, eg
  hw/pcnet.c: Fix EPROM contents to suit AMD netware drivers

and then in the body of the commit message you have more detail.
Remarks about what you're fixing and what you've tested go
here, not in comments like:
// bugfix under DOS with AMD netware driver: AMD PCNTNW Ethernet MLID
v3.10 (960115), network card not found
// works well under DOS with AMD NDIS driver v2.0.1, Knoppix 6.2 ok

If you fix these cosmetic issues you'll have a patch which
is easier to review and more likely to be applied.

-- PMM