qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 2/2 V1] Fixed EPROM for AMD driver compatibility


From: Gerhard Wiesinger
Subject: Re: [Qemu-devel] [PATCH 2/2 V1] Fixed EPROM for AMD driver compatibility under DOS with Netware driver
Date: Wed, 23 Feb 2011 19:06:03 +0100 (CET)
User-agent: Alpine 2.02 (LFD 1266 2009-07-14)

On Wed, 23 Feb 2011, Peter Maydell wrote:

On 22 February 2011 21:00, Gerhard Wiesinger <address@hidden> wrote:
Hello,
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.

I'm not a git expert. Can you explain how to merge 3 commit to one output as expected?

Rest: no problem, will post it.

Ciao,
Gerhard

--
http://www.wiesinger.com/



reply via email to

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