Re: [Qemu-devel] [PATCH v10 3/5] qemu: URI parsing library

[Daniel P. Berrange]

On Thu, Sep 27, 2012 at 05:55:15PM +0200, Paolo Bonzini wrote:
> Il 27/09/2012 16:36, Daniel P. Berrange ha scritto:
> >> > qemu: URI parsing library
> >> > 
> >> > From: Paolo Bonzini <address@hidden>
> >> > 
> >> > Add a new URI parsing library to QEMU. The code has been borrowed from
> >> > libxml2 and libvirt.
> >> > 
> >> > Signed-off-by: Paolo Bonzini <address@hidden>
> >> > Signed-off-by: Bharata B Rao <address@hidden>
> >> > ---
> >> > 
> >> >  Makefile.objs |    2 
> >> >  uri.c         | 2249 
> >> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> >  uri.h         |  113 +++
> > IMHO, you should also be importing the URI code test suite from libvirt
> > to verify that the way you merged/changed the codebases did not break
> > anything.
> 
> Yes, can be done separately though.

In this case I disagree.  The URI parsing code here is complex enough that
I don't think any reviewer can credibly claim to spot flaws that might
have been introduced when combing the libvirt + libxml2 URI code parts.
A test suite is the only way to validate this kind of complex code IMHO
and so should be included in this patch.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|