On 02/27/2013 09:29 PM, TJ wrote:
>> But it didn't add any unit tests
>> (a new .xml file somewhere under tests/networkxml2argvdata or
>> networkxml2xml{in,out} to verify that we can round-trip the new XML and
>> generate the expected command line), and it failed to add the RelaxNG
>> grammar specification under docs/schemas/network.rng, so there is still
>> more to be added.
>
> OK, I'll do those. As you probably realise I'm new to libvirt coding style and requirements, only touching it to add needed functionality, but hopefully can make it suitable for give-back to the
> project. I'll work up a V2 once all comments are in.
I guess I forgot to say one thing up front - while my review may have
sounded negative, that's just because it's easier to be terse when
pointing out how things can be improved. The fact that I replied on the
same day that you posted is a GOOD sign that your work is interesting,
and likely to be accepted once it is whipped into shape. Check out
HACKING, and feel free to ask questions if you have them. You may want
to wait for Laine Stump to also review, as he is more of the expert on
networking related patches, and will have more technical comments as
opposed to my stylistic overview.
And a big THANK YOU for contributing to the community. Too often, I
forget to express gratitude for new contributors braving the unknown
waters of posting to a list where they are not sure of the conventions.
Honestly, we aren't trying to scare you off :)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list