qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: Bin Meng <bmeng.cn@gmail.com>
Cc: Alistair Francis <alistair23@gmail.com>,
	Alistair Francis <alistair.francis@wdc.com>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [PULL 00/18] riscv-to-apply queue
Date: Wed, 26 Aug 2020 10:25:15 +0100	[thread overview]
Message-ID: <CAFEAcA9026WV-t5Q9EbV-nZi+6GZ3WoKCx2tA9QeAXu5iD5ryg@mail.gmail.com> (raw)
In-Reply-To: <CAEUhbmWN93n1JoGszsW6WrkGbdFD6VsGyi7Ji6bS6wF+DbMOBw@mail.gmail.com>

On Wed, 26 Aug 2020 at 04:21, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Wed, Aug 26, 2020 at 6:41 AM Alistair Francis <alistair23@gmail.com> wrote:
> > Richard and Philippe review patches and some of the RISC-V patches get
> > reviewed by the RISC-V community. The main problem (which is a common
> > problem in open source) is that large technical patch series just get
> > ignored.
>
> Yep, I am only comfortable reviewing patches which I have confidence
> in. Right now I am not working on any H- or V - extension for RISC-V
> so I cannot contribute to any review of these large numbers of H- or
> V- extension related patches. Sorry!

So, everybody has a ton of work they need to do and only a limited
amount of time they might have for code review, so it's important to
prioritise. But I would encourage you, and other people contributing
to RISC-V parts of QEMU, to at least sometimes review changes that are
a little bit out of your "comfort zone" if nobody else seems to be
doing so. Review can find bugs, areas that are confusing or need
comments, etc, even without a thorough knowledge of the relevant spec.
(In fact, not knowing the spec can help in identifying where
explanatory comments can help the reader!) And for the project it means
we have more people who at least have some idea of what that bit of code
is doing. Review that is limited to "this code seems to make sense but
I haven't checked it against the spec" is better than patches getting
no review at all, I think. And it's a good way to build your knowledge
of the codebase and the architecture over time.

thanks
-- PMM


  reply	other threads:[~2020-08-26  9:26 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-25 18:48 [PULL 00/18] riscv-to-apply queue Alistair Francis
2020-08-25 18:48 ` [PULL 01/18] hw/riscv: Allow creating multiple instances of CLINT Alistair Francis
2020-08-25 18:48 ` [PULL 02/18] hw/riscv: Allow creating multiple instances of PLIC Alistair Francis
2020-08-25 18:48 ` [PULL 03/18] hw/riscv: Add helpers for RISC-V multi-socket NUMA machines Alistair Francis
2020-08-25 18:48 ` [PULL 04/18] hw/riscv: spike: Allow creating multiple NUMA sockets Alistair Francis
2020-08-25 18:48 ` [PULL 05/18] hw/riscv: virt: " Alistair Francis
2021-08-09  9:46   ` Peter Maydell
2021-08-12 14:57     ` Peter Maydell
2020-08-25 18:48 ` [PULL 06/18] target/riscv: Allow setting a two-stage lookup in the virt status Alistair Francis
2020-08-25 18:48 ` [PULL 07/18] target/riscv: Allow generating hlv/hlvx/hsv instructions Alistair Francis
2020-08-25 18:48 ` [PULL 08/18] target/riscv: Do two-stage lookups on " Alistair Francis
2020-08-25 18:48 ` [PULL 09/18] target/riscv: Don't allow guest to write to htinst Alistair Francis
2020-08-25 18:48 ` [PULL 10/18] target/riscv: Convert MSTATUS MTL to GVA Alistair Francis
2020-08-25 18:48 ` [PULL 11/18] target/riscv: Fix the interrupt cause code Alistair Francis
2020-08-25 18:48 ` [PULL 12/18] target/riscv: Update the Hypervisor trap return/entry Alistair Francis
2020-08-25 18:48 ` [PULL 13/18] target/riscv: Update the CSRs to the v0.6 Hyp extension Alistair Francis
2020-08-25 18:48 ` [PULL 14/18] target/riscv: Only support a single VSXL length Alistair Francis
2020-08-25 18:48 ` [PULL 15/18] target/riscv: Only support little endian guests Alistair Francis
2020-08-25 18:48 ` [PULL 16/18] target/riscv: Support the v0.6 Hypervisor extension CRSs Alistair Francis
2020-08-25 18:48 ` [PULL 17/18] target/riscv: Return the exception from invalid CSR accesses Alistair Francis
2020-08-25 18:48 ` [PULL 18/18] target/riscv: Support the Virtual Instruction fault Alistair Francis
2020-08-25 21:24 ` [PULL 00/18] riscv-to-apply queue Peter Maydell
2020-08-25 21:21   ` Alistair Francis
2020-08-25 21:49     ` Peter Maydell
2020-08-25 22:30       ` Alistair Francis
2020-08-26  3:21         ` Bin Meng
2020-08-26  9:25           ` Peter Maydell [this message]
2020-08-26 10:06             ` Bin Meng
2020-08-27 15:44               ` Alistair Francis
2020-08-29 15:49         ` LIU Zhiwei
2020-08-29 17:30           ` Alistair Francis
2020-08-26  9:28 ` Peter Maydell
2020-10-29 14:13 Alistair Francis
2020-11-01 14:02 ` Peter Maydell
2020-11-01 16:27   ` Bin Meng
2021-10-28  4:43 Alistair Francis
2021-10-28 16:54 ` Richard Henderson

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=CAFEAcA9026WV-t5Q9EbV-nZi+6GZ3WoKCx2tA9QeAXu5iD5ryg@mail.gmail.com \
    --to=peter.maydell@linaro.org \
    --cc=alistair.francis@wdc.com \
    --cc=alistair23@gmail.com \
    --cc=bmeng.cn@gmail.com \
    --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).