qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: amagdy.afifi@gmail.com, qemu-devel@nongnu.org
Cc: qemu-riscv@nongnu.org, sagark@eecs.berkeley.edu,
	kbastian@mail.uni-paderborn.de, palmer@sifive.com,
	mjc@sifive.com, Alistair.Francis@wdc.com
Subject: Re: [Qemu-devel] [PATCH] riscv: Add proper alignment check and pending 'C' extension upon misa writes
Date: Sat, 23 Feb 2019 15:45:59 -0600	[thread overview]
Message-ID: <8f36a26e-7e20-1b9c-18bd-a5ce4b3d9c14@redhat.com> (raw)
In-Reply-To: <20190222162555.13764-2-amagdy.afifi@gmail.com>

On 2/22/19 10:25 AM, amagdy.afifi@gmail.com wrote:
> From: ahmed_magdy <amagdy.afifi@gmail.com>
> 
> Signed-off-by: ahmed_magdy <amagdy.afifi@gmail.com>

This appears to be your first contribution to qemu. Welcome to the
community!

Typically, a Signed-off-by designation should be a proper name (what you
would sign a legal document with, as it has a legal significance on your
right to contribute the code).  Using all lowercase and _ instead of
space looks like a username, and while I am not one to tell you it can't
be a legal name, it is unusual enough to at least raise my suspicions.

Furthermore, your commit message doesn't give any details beyond the
"what" in the subject line. The body of the commit message should
explain the "why" (what bug are you fixing, how to reproduce it), so
that a reviewer stands a chance of determining if the code matches the
description you gave, and if the issue you describe really does warrant
the inclusion of your patch.  You gave a brief "why" in your cover letter:

"I'm submiting this patch to properly check the next instruction
alignment and scheduale compression extenstion enable upon 'MISA'
register writes to later aligned instruction through exporting next
instruction 'pc' to riscv cpu state"

where it would be wise to include an improved version of that text with
this commit proper (since the cover letter does not get applied to git).
 For that matter, when sending a single patch, a cover letter is
optional (it is only mandatory when sending a multi-patch series).

For more patch submission hints, see:
https://wiki.qemu.org/Contribute/SubmitAPatch

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

  parent reply	other threads:[~2019-02-23 21:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-22 16:25 [Qemu-devel] Add proper alignment check and pending 'C' extension for riscv amagdy.afifi
2019-02-22 16:25 ` [Qemu-devel] [PATCH] riscv: Add proper alignment check and pending 'C' extension upon misa writes amagdy.afifi
2019-02-22 23:57   ` Richard Henderson
2019-02-24  7:57     ` Amed Magdy
2019-02-24 19:04       ` Richard Henderson
2019-02-26  7:58         ` Amed Magdy
2019-02-26  8:11           ` Amed Magdy
2019-02-23 21:45   ` Eric Blake [this message]
2019-02-24  8:07     ` Amed Magdy
2019-02-25 14:14       ` Eric Blake

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=8f36a26e-7e20-1b9c-18bd-a5ce4b3d9c14@redhat.com \
    --to=eblake@redhat.com \
    --cc=Alistair.Francis@wdc.com \
    --cc=amagdy.afifi@gmail.com \
    --cc=kbastian@mail.uni-paderborn.de \
    --cc=mjc@sifive.com \
    --cc=palmer@sifive.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=sagark@eecs.berkeley.edu \
    /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).