Re: [PATCH v1 05/26] qemu_domain_address: Reformat qemuDomainAssignS390Addresses()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 30/11/2020 11.18, Michal Privoznik wrote:
> On 11/30/20 10:38 AM, Thomas Huth wrote:
>> On 27/11/2020 16.02, Michal Privoznik wrote:
>>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>>> ---
>>>   src/qemu/qemu_domain_address.c | 10 ++++------
>>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
>>> index 2788dc7fb3..d872f75b38 100644
>>> --- a/src/qemu/qemu_domain_address.c
>>> +++ b/src/qemu/qemu_domain_address.c
>>> @@ -408,18 +408,16 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def,
>>>       if (qemuDomainIsS390CCW(def) &&
>>>           virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCW)) {
>>>           if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW))
>>> -            qemuDomainPrimeVfioDeviceAddresses(
>>> -                def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
>>> -        qemuDomainPrimeVirtioDeviceAddresses(
>>> -            def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
>>> +            qemuDomainPrimeVfioDeviceAddresses(def,
>>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
>>
>> Looks fine to me, but docs/coding-style.rst still suggest to format code
>> with "indent -l75", so is this really the right thing to do here?
> 
> It's true that we have 80 characters limit, but that is more of a soft limit
> and common sense should be used. Personally, I find
> 
>    func(
> arg1, arg2
> );
> 
> worse than exceeding that 80 char rule. My common sense tells me that the
> rule tries to avoid the following pattern (among others):
> 
>   func(arg1, arg2, ...., very_long_list_of_arguments, which, could, easily,
> go_on_multiple_lines, but, didnt);

Fair point, but then this should IMHO be reflected in the coding-style doc
first. Otherwise the next contributor to this file might simply undo your
change to fit everything again into the 80 (or even 75) columns limit...?

 Thomas




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux