On Sun, 2015-05-31 at 15:19 -0400, John Ferlan wrote:
First of all, thanks for the review :)
> Patch 2 had some comments which should be simple to fix
I've commented in detail in your other mail, should have taken care of
them all.
> Patch 4 had a couple of NIT's about going beyond 80 columns on a line,
> but since it's so pervasive in the rest of the module that can be a
> future cleanup ;-)...
Sure, there's always time for cleaning up after the cleanup :)
> I do note that 'vshCommandOpt' now has an
> unrelated difference - I assume it had a ctl argument at one point
> that's since been removed...
I'm not sure what you mean here, it looks fine to me. Care to explain in
more detail?
> Patch 5 still seemed to need some sort of error message in
> vshCommandOptString... Some callers ignore return status, so even adding
> an error may still be "ignored".
This series deals with numeric options only. String options are
something I will probably tackle in the near future :)
> Overall seems OK to me with some minor cleanups.
Looks like this series is growing a new commit for every subsequent
review! I've just posted v4, which should address your comments. Please
take a look at it and let me know if there's more work to do.
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team
$ python -c "print('a'.join(['', 'bologn', '@redh', 't.com']))"
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list