[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v4 4/5] iotests: Add VMDK tests for
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v4 4/5] iotests: Add VMDK tests for blockdev-create |
Date: |
Fri, 7 Dec 2018 18:09:40 +0100 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
Am 07.12.2018 um 17:52 hat Eric Blake geschrieben:
> On 12/7/18 10:42 AM, Kevin Wolf wrote:
> > Am 07.12.2018 um 16:40 hat Eric Blake geschrieben:
> > > On 12/7/18 8:45 AM, Kevin Wolf wrote:
> > > > Am 07.12.2018 um 14:12 hat Markus Armbruster geschrieben:
> > > > > git-am complains
> > > > >
> > > > > Applying: iotests: Add VMDK tests for blockdev-create
> > > > > .git/rebase-apply/patch:281: trailing whitespace.
> > > > > format:
> > > > > .git/rebase-apply/patch:308: trailing whitespace.
> > > > > format:
> > > > > .git/rebase-apply/patch:335: trailing whitespace.
> > > > > format:
> > > > > .git/rebase-apply/patch:600: new blank line at EOF.
> > > > > +
> > > > > warning: 4 lines add whitespace errors.
> > > >
> > > > This is in the reference output, so trailing whitespace/blank lines are
> > > > actually correct.
> > >
> > > Ah, but doesn't ./check already ignore differences in trailing whitespace
> > > present in the actual running that is not present in the *.out files,
> > > precisely so we don't have to check in trailing whitespace reference
> > > outputs?
> >
> > It does ignore whitespace changes, so even if we remove that whitespace,
> > the test won't fail. But I don't think that's a good reason to check in
> > inaccurate reference output.
> >
> > There are a few test cases that have a reference output like this and
> > it's always annoying: When I later add a new subtest, I add the new test
> > code, review the ./check output and if it looks good, I do something
> > like 'cp 237.out.bad 237.out'. At that point, I'll have to manually
> > revert completely unrelated whitespace changes again.
>
> Is it worth teaching iotests.img_info_log() to strip trailing whitespace?
> Or even to teach qemu-img itself to quit generating trailing whitespace?
Not sure if it's worth it, but if someone proposed such a change, I
wouldn't object anyway.
> > Some 'git am' warnings feel like the lesser evil to me.
>
> True, and a comment in the commit message about intentionally triggering a
> known checkpatch flag goes a long ways to document why you want the trailing
> whitespace (assuming we don't instead decide to tackle a root cause of
> having the whitespace in the first place).
Fair enough. Once it's merged, it doesn't make a difference any more,
but it could have made review a bit easier.
Kevin
- Re: [Qemu-block] [Qemu-devel] [PATCH v4 2/5] vmdk: Implement .bdrv_co_create callback, (continued)
[Qemu-block] [PATCH v4 5/5] vmdk: Reject excess extents in blockdev-create, Kevin Wolf, 2018/12/07
Re: [Qemu-block] [Qemu-devel] [PATCH v4 0/5] vmdk: Implement blockdev-create, no-reply, 2018/12/07