Hi Christophe, On Tue, Dec 3, 2013 at 5:12 AM, Christophe Fergeau <cferg...@redhat.com>wrote:
> Thanks for the patch, a few quick initial comments: > - formatting of the commit log was off, see (for example) > > https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_GIT_commit_message_structure > for the recommended format of git commit logs (helps when using 'git > shortlog' for example) > This document as a whole is an interesting read imo ;) > Thanks for the tip! > - I'd use ovirt_vmpool_allocate_vm (rather than _allocatevm) > I used allocatevm because the action is actually "allocatevm" not "allocate_vm", but if you think the function name doesn't need to be in line with the action, then I agree. > - imo the ovirt_invoke_action stuff should be moved to a generic place > before this patch, but I can look into that > Yes, it's basically duplicated code, but I didn't think it was up to me to restructure :). > - there were a few minor white space issues (see patch below) > My editor was misconfigured... > - you need to export the new public functions in govirt/govirt.sym (see > patch below) > Yes, I meant to ask you about that, but I wasn't sure what version to put, so I left it up to you (which was a good idea I think :)) Would you like me to generate a patch with all the suggested changes and a better log entry? Cheers! iordan -- The conscious mind has only one thread of execution.
_______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel