[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: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH V7 04/14] block: move collect_snapshots() and collect_image_info() to block.c |
Date: |
Thu, 28 Feb 2013 16:24:25 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Feb 27, 2013 at 10:32:13AM +0800, Wenchao Xia wrote:
> δΊ 2013-2-26 23:59, Eric Blake ει:
> >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.
> >
> OK, I'll add it to the commit message about the patch.
I would go ahead and rename to "bdrv_..." in this patch. I was about to
leave a comment asking you to rename it when I saw Eric's reply.
Stefan
[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
[Qemu-devel] [PATCH V7 06/14] block: add image info query function bdrv_query_image_info(), Wenchao Xia, 2013/02/26