Re: [PATCH 0/3] Clean up qemuhotplugtest xml files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 08, 2016 at 12:52:07AM +0200, Tomasz Flendrich wrote:
I plan on doing bigger changes to qemuhotplugtest.c. It's going
to test device attachment/detachment to both live (like now) and
persistent (something that is missing now) domains. More files
will be added, so I decided to clean up ones that are there now.

Many files were not even connected to any qemuxml2argv test,
and yet they were in tests/qemuxml2argvdata/

Tomasz Flendrich (3):
 Move all qemuhotplugtest XMLs to one directory
 Move domain and device xmls to different directories
 Use virschematest on qemuhotplugtest domain xmls


So I found out there is no need for base+qemu-agent(-detach).xml
(without the '-live') part and I was looking for a place where you
added.  After some time I've found it is not connected to your patches
at all =)

Anyway, the patches look fine apart from two things:

1) They were sent a bit weirdly, but you probably noticed that they are
   not threaded properly, right?  Since you sent another series that's
   fine I don't even feel like I need to suggest you to check your
   settings.

2) There are no descriptions in the commits.  Some more info in the
   commit message might help others find out why that specific change
   was done when looking at it few years from now.

No. 1 is not blocking the inclusion and I'll fix no. 2 this time.  Just
consider describing the changes next time.  Feel free to check what I
added there to have an idea how such message could look like.  Also I
think 3/3 can be merged together with 2/3 as it explains the intention
more nicely.  With that said ACK series, I'll push it in a while.

Martin

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]