Re: [Xen-devel] [PULL v2 0/3] Trivial branch patches

[Laurent Vivier]

Le 06/11/2019 à 17:05, Peter Maydell a écrit :
> On Tue, 5 Nov 2019 at 20:06, Laurent Vivier <address@hidden> wrote:
>>
>> Le 05/11/2019 à 20:20, address@hidden a écrit :
>>> Patchew URL: https://patchew.org/QEMU/address@hidden/
>>>
>>>
>>>
>>> Hi,
>>>
>>> This series seems to have some coding style problems. See output below for
>>> more information:
>>>
>>> Subject: [Xen-devel] [PULL v2 0/3] Trivial branch patches
>>> Type: series
>>> Message-id: address@hidden
>>>
>>> === TEST SCRIPT BEGIN ===
>>> #!/bin/bash
>>> git rev-parse base > /dev/null || exit 0
>>> git config --local diff.renamelimit 0
>>> git config --local diff.renames True
>>> git config --local diff.algorithm histogram
>>> ./scripts/checkpatch.pl --mailback base..
>>> === TEST SCRIPT END ===
>>>
>>> Switched to a new branch 'test'
>>> 49a55f7 global: Squash 'the the'
>>> c0b5513 hw/misc/grlib_ahb_apb_pnp: Fix 8-bit accesses
>>> eb43395 hw/misc/grlib_ahb_apb_pnp: Avoid crash when writing to PnP registers
>>>
>>> === OUTPUT BEGIN ===
>>> 1/3 Checking commit eb43395bf8f1 (hw/misc/grlib_ahb_apb_pnp: Avoid crash 
>>> when writing to PnP registers)
>>> 2/3 Checking commit c0b5513f971a (hw/misc/grlib_ahb_apb_pnp: Fix 8-bit 
>>> accesses)
>>> 3/3 Checking commit 49a55f7feb19 (global: Squash 'the the')
>>> ERROR: do not use C99 // comments
>>> #26: FILE: disas/libvixl/vixl/invalset.h:105:
>>> +  // Note that this does not mean the backing storage is empty: it can 
>>> still
>>
>> As reported by David Gilbert, this is a false positive as this file is a
>> C++ file.
> 
> True, but it's also a third-party dependency that we have
> taken a copy of. If we care about fixing its typos in comments
> we should do that by submitting the fixes upstream. We should
> avoid making changes to our local copy of this code because
> it just makes it unnecessarily more difficult to do updates to
> newer versions.
> 
> Could you drop that change, please? (Sorry I didn't notice
> it going by when the patch was originally on list.)

Sure.

Thanks,
Laurent