[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
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH V7 03/14] block: return bool for bdrv_can_snapshot(), Wenchao Xia, 2013/02/26
[Qemu-devel] [PATCH V7 05/14] block: add snapshot info query function bdrv_query_snapshot_infolist(), Wenchao Xia, 2013/02/26
Re: [Qemu-devel] [PATCH V7 05/14] block: add snapshot info query function bdrv_query_snapshot_infolist(), Stefan Hajnoczi, 2013/02/28