From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60814) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gVJdP-0003wW-CI for qemu-devel@nongnu.org; Fri, 07 Dec 2018 12:09:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gVJdO-00032g-HK for qemu-devel@nongnu.org; Fri, 07 Dec 2018 12:09:51 -0500 Date: Fri, 7 Dec 2018 18:09:40 +0100 From: Kevin Wolf Message-ID: <20181207170940.GI5119@linux.fritz.box> References: <20181207115343.6747-1-kwolf@redhat.com> <20181207115343.6747-5-kwolf@redhat.com> <87in05wkev.fsf@dusky.pond.sub.org> <20181207144519.GF5119@linux.fritz.box> <5c0dd2fb-fbe6-41da-77ff-def0f2f3e830@redhat.com> <20181207164209.GH5119@linux.fritz.box> <77a60e02-c5e5-4f30-46fd-3eca4604f038@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <77a60e02-c5e5-4f30-46fd-3eca4604f038@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 4/5] iotests: Add VMDK tests for blockdev-create List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Markus Armbruster , qemu-devel@nongnu.org, qemu-block@nongnu.org, mreitz@redhat.com 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