[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] QemuOpt: add unit tests
From: |
Leandro Dorileo |
Subject: |
Re: [Qemu-devel] [PATCH v3] QemuOpt: add unit tests |
Date: |
Mon, 19 May 2014 18:31:02 -0300 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Wed, Apr 30, 2014 at 10:28:26AM +0200, Markus Armbruster wrote:
> Leandro Dorileo <address@hidden> writes:
>
> > Hi Andreas,
> >
> > On Mon, Apr 28, 2014 at 09:02:12PM +0200, Andreas Färber wrote:
> >> Am 28.04.2014 20:55, schrieb Leandro Dorileo:
> >> > ping?
> >> >
> >> >
> >> > On Tue, Mar 25, 2014 at 10:27:19AM -0300, Leandro Dorileo wrote:
> >> >> Cover basic aspects and API usage for QemuOpt. The current
> >> >> implementation
> >> >> covers the API's planned to be changed by Chunyan Liu in his
> >> >> QEMUOptionParameter
> >> >> replacement/cleanup job.
> >> >>
> >> >> Other APIs should be covered in future improvements.
> >> >>
> >> >> Signed-off-by: Leandro Dorileo <address@hidden>
> >> >> Reviewed-by: Eric Blake <address@hidden>
> >> >> ---
> >> >> tests/Makefile | 3 +
> >> >> tests/test-qemu-opts.c | 455
> >> >> +++++++++++++++++++++++++++++++++++++++++++++++++
> >> >> 2 files changed, 458 insertions(+)
> >> >> create mode 100644 tests/test-qemu-opts.c
> >>
> >> I don't spot any test API or style issues; not familiar enough with
> >> QemuOpts to judge the tests though.
> >
> > What maintainer would best suite to route this patch?
>
> MAINTAINERS doesn't cover it. In the past year, patches went in via
> Anthony, Kevin, Luiz, or Stefan Hajnoczi. I'm cc'ing the ones from this
> list you missed.
Ok.
>
> I read your patch. There's some redundant testing, some comments feel
> superfluous, and there's a spelling mistake or two. Coverage is far
> from perfect (QemuOpts has become ridiculously complex), but infinitely
> better than before :)
>
Redundant testing don't seem to be a problem in this case, the comments are
intentionally verbose. My spell checker doesn't spot any spelling error -
besides "defval" :)
> Let's take it as is.
I'm respining the patch changing what Stefan suggested.
Thanks...
--
Leandro Dorileo