[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 1/9] qemu-binfmt-conf.sh: enforce safe style consistency
From: |
Eric Blake |
Subject: |
Re: [PATCH v8 1/9] qemu-binfmt-conf.sh: enforce safe style consistency |
Date: |
Mon, 9 Mar 2020 13:32:35 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
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