qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gerhard Wiesinger <lists@wiesinger.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org
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)	[thread overview]
Message-ID: <alpine.LFD.2.02.1102231904480.28972@bbs.intern> (raw)
In-Reply-To: <AANLkTik4ujGrwSanLL1LJJU7q=PLwXSp1xdkEWGAVX9g@mail.gmail.com>

On Wed, 23 Feb 2011, Peter Maydell wrote:

> On 22 February 2011 21:00, Gerhard Wiesinger <lists@wiesinger.com> 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	other threads:[~2011-02-23 18:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-20 19:53 [Qemu-devel] [PATCH 2/2 V1] Fixed EPROM for AMD driver compatibility under DOS with Netware driver Gerhard Wiesinger
2011-02-22 21:00 ` Gerhard Wiesinger
2011-02-23  8:43   ` Peter Maydell
2011-02-23 18:06     ` Gerhard Wiesinger [this message]
2011-02-23 18:25       ` Peter Maydell
2011-02-23 20:24         ` Gerhard Wiesinger
2011-03-05 12:48           ` Gerhard Wiesinger
2011-03-06 18:13             ` Andreas Färber

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=alpine.LFD.2.02.1102231904480.28972@bbs.intern \
    --to=lists@wiesinger.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).