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
