Re: [kvm-unit-tests PATCH v3 0/5] add shellcheck support

[Thomas Huth <thuth@redhat.com>]

On 03/05/2024 07.02, Nicholas Piggin wrote:
> On Thu May 2, 2024 at 7:34 PM AEST, Thomas Huth wrote:
>> On 02/05/2024 10.56, Andrew Jones wrote:
>>> On Thu, May 02, 2024 at 10:23:22AM GMT, Thomas Huth wrote:
>>>> On 01/05/2024 13.29, Nicholas Piggin wrote:
>>>>> This is based on upstream directly now, not ahead of the powerpc
>>>>> series.
>>>>
>>>> Thanks! ... maybe you could also rebase the powerpc series on this now? (I
>>>> haven't forgotten about it, just did not find enough spare time for more
>>>> reviewing yet)
>>>>
>>>>> Since v2:
>>>>> - Rebased to upstream with some patches merged.
>>>>> - Just a few comment typos and small issues (e.g., quoting
>>>>>      `make shellcheck` in docs) that people picked up from the
>>>>>      last round.
>>>>
>>>> When I now run "make shellcheck", I'm still getting an error:
>>>>
>>>> In config.mak line 16:
>>>> AR=ar
>>>> ^-- SC2209 (warning): Use var=$(command) to assign output (or quote to
>>>> assign string).
>>>
>>> I didn't see this one when testing. I have shellcheck version 0.9.0.
>>
>> I'm also using 0.9.0 (from Fedora). Maybe we've got a different default config?
> 
> I have 0.10.0 from Debian with no changes to config defaults and no
> warning.

If I understood it correctly, it warns for AR=ar but it does not warn for 
AR=powerpc-linux-gnu-ar ... could you try the first term, too, if you 
haven't done so yet?

>> Anyway, I'm in favor of turning this warning of in the config file, it does
>> not seem to be really helpful in my eyes. What do you think?
> 
> Maybe it would be useful. I don't mind quoting strings usually, although
> for this kind of pattern it's a bit pointless and config.mak is also
> Makefile so that has its own issues. Maybe just disable it for this
> file?

Yes, either for this file only, or globally ... I don't mind. Could you send 
a patch, please?

  Thanks,
   Thomas