Re: [Qemu-devel] [PATCH v1 01/24] qmp: Extract system emulation related code from qmp.c into qmp-system.c

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

On 07/31/2014 11:26 PM, Benoît Canet wrote:
> This patch will allow to link qmp.o with utility binaries without dragging too

s/allow to link/allow linking/

> much unrelated object files and externals dependencies.

s/dragging too much/dragging in too many/
s/externals/external/

> 
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  Makefile.objs |   2 +-
>  qmp-system.c  | 376 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qmp.c         | 361 +------------------------------------------------------
>  3 files changed, 379 insertions(+), 360 deletions(-)
>  create mode 100644 qmp-system.c
> 

When reviewing code motion, I like to do:

$ diff -u <(sed -n 's/^-//p' patch) <(sed -n 's/^+//p' patch)

In the case of this patch, you rearranged functions, which makes it MUCH
harder to see if everything moved correctly (for example, the old code
has qmp_query_kvm, qmp_query_uuid, qmp_quit, qmp_stop...; the new code
has them in a different order).  It would be a lot easier if you create
the new file with the function order preserved as it was in the original
file.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]