Re: [Qemu-devel] [PATCH v3] QemuOpt: add unit tests

[Leandro Dorileo <l@dorileo.org>]

On Wed, Apr 30, 2014 at 10:28:26AM +0200, Markus Armbruster wrote:
> Leandro Dorileo <l@dorileo.org> 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 <l@dorileo.org>
> >> >> Reviewed-by: Eric Blake <eblake@redhat.com>
> >> >> ---
> >> >>  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