qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH V7 04/14] block: move collect_snapshots() and co


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH V7 04/14] block: move collect_snapshots() and collect_image_info() to block.c
Date: Tue, 26 Feb 2013 08:59:01 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130219 Thunderbird/17.0.3

On 02/26/2013 03:40 AM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia <address@hidden>

In v6, I complained that these functions didn't match the namespace
expected in block.c.  You replied:

>>   This patch is just for making review easier, those two functions will
>> be deleted later so they do not have much meaning, renaming may bring
>> confusion to reviewer.

This information needs to be in the commit message, so that someone that
happens on this commit during a git bisect will understand that several
patches later gets rid of the problems.  In other words, any commit that
temporarily violates coding standards with plans to fix things up later
should document that fact, rather than making reviewers chase down
conversations in the mailing list.

Personally, I think that renaming during code motion is not that
confusing; but as I'm not the maintainer of this code, you might want to
get Kevin or Stefan's opinion before spending your time to add churn
that they don't find necessary.

> ---
>  block.c               |   81 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/block/block.h |    4 ++
>  qemu-img.c            |   81 
> -------------------------------------------------
>  3 files changed, 85 insertions(+), 81 deletions(-)

I have reviewed that this is a straight code motion patch, so if you
have to respin the series for other reasons, and all that changes on
this patch is adding a proper commit message, then I'm okay if you add:
Reviewed-by: Eric Blake <address@hidden>

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]