Re: [Qemu-block] [PATCH v6 04/15] iotests: Move _filter_nbd into common.filter

[Kevin Wolf]

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.

Kevin

pgpgWB4PtuISR.pgp
Description: PGP signature