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

[Peter Maydell <peter.maydell@linaro.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