[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 04/15] iotests: Move _filter_nbd into common.
Re: [Qemu-block] [PATCH v6 04/15] iotests: Move _filter_nbd into common.filter
Tue, 10 Nov 2015 11:25:44 +0100
Am 09.11.2015 um 19:17 hat Max Reitz geschrieben:
> On 09.11.2015 17:04, Kevin Wolf wrote:
> > Am 04.11.2015 um 19:57 hat Max Reitz geschrieben:
> >> _filter_nbd can be useful for other NBD tests, too, therefore it should
> >> reside in common.filter, and it should support URLs of the "nbd://"
> >> format and export names.
> >> The NBD log lines ("/your/source/dir/nbd.c:function():line: error")
> >> should not be converted to empty lines but removed altogether.
> >> Signed-off-by: Max Reitz <address@hidden>
> > Code motion and modification in the same patch is bad style. The changes
> > look good, though.
> Considering splitting this into two patches will result basically in
> both of them each changing just as much as this single patch does
> (because test 083 uses tabs instead of spaces) I'm inclined to just
> change the commit title to "Remove filter_nbd and add _filter_nbd" instead.
You're confusing "changing much" with "touching many lines". What I'm
asking is to split this in two: One mostly mechanical patch that doesn't
change semantics, and one patch for the functional change. This makes it
much easier to spot the functional changes that are actually made.
For example, whitespace changes during code motion are not a problem at
all. I use git show -w routinely to review those. I can also cope with
other minor things like style changes during code motion.
The hard part during review is just finding the 10% of actual functional
change in the middle of the 90% that change nothing semantically,
especially if multiple hunks are involved in the functional change.
Description: PGP signature
[Qemu-block] [PATCH v6 05/15] iotests: Make redirecting qemu's stderr optional, Max Reitz, 2015/11/04
[Qemu-block] [PATCH v6 06/15] iotests: Add test for eject under NBD server, Max Reitz, 2015/11/04
[Qemu-block] [PATCH v6 07/15] block: Move BDS close notifiers into BB, Max Reitz, 2015/11/04
[Qemu-block] [PATCH v6 08/15] block: Use blk_remove_bs() in blk_delete(), Max Reitz, 2015/11/04
[Qemu-block] [PATCH v6 09/15] blockdev: Use blk_remove_bs() in do_drive_del(), Max Reitz, 2015/11/04
[Qemu-block] [PATCH v6 10/15] block: Make bdrv_close() static, Max Reitz, 2015/11/04