Re: [PATCH v8 1/9] qemu-binfmt-conf.sh: enforce safe style consistency

[Eric Blake <eblake@redhat.com>]

On 3/9/20 1:20 PM, Unai Martinez Corral wrote:
>   2020/03/09 16:01, Eric Blake:
> 
>> On 3/7/20 11:22 AM, Unai Martinez-Corral wrote:
>>> Spaces are added before '; then', for consistency.
>>
>> For consistency with what?  This is not our prevailing style; as
>> evidenced by this pre-patch search:
>>
>> $ git grep 'if \[.*\];' | wc
>>       274    2186   18170
>> $ git grep 'if \[.*\] ;' | wc
>>        25     256    1573
>>
>> and you are diverging from the dominant pattern.
>>
> 
> For consistency within the script that is being modified. I'm not trying to
> diverge, neither do I prefer any specific style.

Aha, I see what you were looking at: within the script itself, it was 10 
'] ;' vs. 2 '];'.  In which case, I'd recommend swapping the 10 
instances over to be common with the rest of the code base, rather than 
the 2 away from the rest of the code base but towards the rest of the 
script.

> Although the style in the current master is not consistent, ' ; ' is
> significantly more frequent. When I was told to keep consistency in v2, I
> picked that because it was the most common.
> Anyway, I will push a new version where all these are changed to the
> dominant pattern outside of this script.

Good to hear.

> 
> 
>> This part, however, is good.  Since one part is controversial, you may
>> want to split this into two patches, or even drop the reformatting part.
>>
> 
> Since the current master is neither consistent nor coherent with the
> dominant pattern, I don't think I can drop the reformatting as long as I
> want to fulfill your requirements.

Splitting into two patches (one to fix '] ;' spacing, the other to add 
'[ "x$..."' protection) is then the best course of action.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org