mbox series

[RFC,kvm-unit-tests,v2,00/14] add shellcheck support

Message ID 20240406123833.406488-1-npiggin@gmail.com (mailing list archive)
Headers show
Series add shellcheck support | expand

Message

Nicholas Piggin April 6, 2024, 12:38 p.m. UTC
Tree here

https://gitlab.com/npiggin/kvm-unit-tests/-/tree/shellcheck

Again on top of the "v8 migration, powerpc improvements" series. I
don't plan to rebase the other way around since it's a lot of work.
So this is still in RFC until the other big series gets merged.

Thanks to Andrew for a lot of review. A submitted the likely s390x
bugs separately ahead of this series, and also disabled one of the
tests and dropped its fix patch as-per review comments. Hence 3 fewer
patches. Other than that, since last post:

* Tidied commit messages and added some of Andrew's comments.
* Removed the "SC2034 unused variable" blanket disable, and just
  suppressed the config.mak and a couple of other warnings.
* Blanket disabled "SC2235 Use { ..; } instead of (..)" and dropped
  the fix for it.
* Change warning suppression comments as per Andrew's review, also
  mention in the new unittests doc about the "check =" option not
  allowing whitespace etc in the name since we don't cope with that.

Thanks,
Nick

Nicholas Piggin (14):
  Add initial shellcheck checking
  shellcheck: Fix SC2223
  shellcheck: Fix SC2295
  shellcheck: Fix SC2094
  shellcheck: Fix SC2006
  shellcheck: Fix SC2155
  shellcheck: Fix SC2143
  shellcheck: Fix SC2013
  shellcheck: Fix SC2145
  shellcheck: Fix SC2124
  shellcheck: Fix SC2294
  shellcheck: Fix SC2178
  shellcheck: Fix SC2048
  shellcheck: Suppress various messages

 .shellcheckrc           | 30 ++++++++++++++++++++++++
 Makefile                |  4 ++++
 README.md               |  3 +++
 arm/efi/run             |  4 ++--
 configure               |  2 ++
 riscv/efi/run           |  4 ++--
 run_tests.sh            | 11 +++++----
 scripts/arch-run.bash   | 52 ++++++++++++++++++++++++++++++-----------
 scripts/common.bash     |  5 +++-
 scripts/mkstandalone.sh |  4 +++-
 scripts/runtime.bash    | 14 +++++++----
 11 files changed, 105 insertions(+), 28 deletions(-)
 create mode 100644 .shellcheckrc

Comments

Thomas Huth April 15, 2024, 11:59 a.m. UTC | #1
On 06/04/2024 14.38, Nicholas Piggin wrote:
> Tree here
> 
> https://gitlab.com/npiggin/kvm-unit-tests/-/tree/shellcheck
> 
> Again on top of the "v8 migration, powerpc improvements" series. I
> don't plan to rebase the other way around since it's a lot of work.
> So this is still in RFC until the other big series gets merged.
> 
> Thanks to Andrew for a lot of review. A submitted the likely s390x
> bugs separately ahead of this series, and also disabled one of the
> tests and dropped its fix patch as-per review comments. Hence 3 fewer
> patches. Other than that, since last post:
> 
> * Tidied commit messages and added some of Andrew's comments.
> * Removed the "SC2034 unused variable" blanket disable, and just
>    suppressed the config.mak and a couple of other warnings.
> * Blanket disabled "SC2235 Use { ..; } instead of (..)" and dropped
>    the fix for it.
> * Change warning suppression comments as per Andrew's review, also
>    mention in the new unittests doc about the "check =" option not
>    allowing whitespace etc in the name since we don't cope with that.
> 
> Thanks,
> Nick
> 
> Nicholas Piggin (14):
>    Add initial shellcheck checking
>    shellcheck: Fix SC2223
>    shellcheck: Fix SC2295
>    shellcheck: Fix SC2094
>    shellcheck: Fix SC2006
>    shellcheck: Fix SC2155
>    shellcheck: Fix SC2143
>    shellcheck: Fix SC2013
>    shellcheck: Fix SC2145
>    shellcheck: Fix SC2124
>    shellcheck: Fix SC2294
>    shellcheck: Fix SC2178
>    shellcheck: Fix SC2048
>    shellcheck: Suppress various messages

I went ahead and pushed a bunch of your patches to the k-u-t master branch 
now. However, there were also some patches which did not apply cleanly to 
master anymore, so please rebase the remaining patches and then send them again.

  Thanks!
   Thomas
Nicholas Piggin April 16, 2024, 3:26 a.m. UTC | #2
On Mon Apr 15, 2024 at 9:59 PM AEST, Thomas Huth wrote:
> On 06/04/2024 14.38, Nicholas Piggin wrote:
> > Tree here
> > 
> > https://gitlab.com/npiggin/kvm-unit-tests/-/tree/shellcheck
> > 
> > Again on top of the "v8 migration, powerpc improvements" series. I
> > don't plan to rebase the other way around since it's a lot of work.
> > So this is still in RFC until the other big series gets merged.
> > 
> > Thanks to Andrew for a lot of review. A submitted the likely s390x
> > bugs separately ahead of this series, and also disabled one of the
> > tests and dropped its fix patch as-per review comments. Hence 3 fewer
> > patches. Other than that, since last post:
> > 
> > * Tidied commit messages and added some of Andrew's comments.
> > * Removed the "SC2034 unused variable" blanket disable, and just
> >    suppressed the config.mak and a couple of other warnings.
> > * Blanket disabled "SC2235 Use { ..; } instead of (..)" and dropped
> >    the fix for it.
> > * Change warning suppression comments as per Andrew's review, also
> >    mention in the new unittests doc about the "check =" option not
> >    allowing whitespace etc in the name since we don't cope with that.
> > 
> > Thanks,
> > Nick
> > 
> > Nicholas Piggin (14):
> >    Add initial shellcheck checking
> >    shellcheck: Fix SC2223
> >    shellcheck: Fix SC2295
> >    shellcheck: Fix SC2094
> >    shellcheck: Fix SC2006
> >    shellcheck: Fix SC2155
> >    shellcheck: Fix SC2143
> >    shellcheck: Fix SC2013
> >    shellcheck: Fix SC2145
> >    shellcheck: Fix SC2124
> >    shellcheck: Fix SC2294
> >    shellcheck: Fix SC2178
> >    shellcheck: Fix SC2048
> >    shellcheck: Suppress various messages
>
> I went ahead and pushed a bunch of your patches to the k-u-t master branch 
> now. However, there were also some patches which did not apply cleanly to 
> master anymore, so please rebase the remaining patches and then send them again.

Hey Thomas,

Yeah the sc patches were based on top of the big series, so some
collisions expected. I'll look at rebasing.

Thanks,
Nick
Thomas Huth April 16, 2024, 4:46 a.m. UTC | #3
On 16/04/2024 05.26, Nicholas Piggin wrote:
> On Mon Apr 15, 2024 at 9:59 PM AEST, Thomas Huth wrote:
>> On 06/04/2024 14.38, Nicholas Piggin wrote:
>>> Tree here
>>>
>>> https://gitlab.com/npiggin/kvm-unit-tests/-/tree/shellcheck
>>>
>>> Again on top of the "v8 migration, powerpc improvements" series. I
>>> don't plan to rebase the other way around since it's a lot of work.
>>> So this is still in RFC until the other big series gets merged.
>>>
>>> Thanks to Andrew for a lot of review. A submitted the likely s390x
>>> bugs separately ahead of this series, and also disabled one of the
>>> tests and dropped its fix patch as-per review comments. Hence 3 fewer
>>> patches. Other than that, since last post:
>>>
>>> * Tidied commit messages and added some of Andrew's comments.
>>> * Removed the "SC2034 unused variable" blanket disable, and just
>>>     suppressed the config.mak and a couple of other warnings.
>>> * Blanket disabled "SC2235 Use { ..; } instead of (..)" and dropped
>>>     the fix for it.
>>> * Change warning suppression comments as per Andrew's review, also
>>>     mention in the new unittests doc about the "check =" option not
>>>     allowing whitespace etc in the name since we don't cope with that.
>>>
>>> Thanks,
>>> Nick
>>>
>>> Nicholas Piggin (14):
>>>     Add initial shellcheck checking
>>>     shellcheck: Fix SC2223
>>>     shellcheck: Fix SC2295
>>>     shellcheck: Fix SC2094
>>>     shellcheck: Fix SC2006
>>>     shellcheck: Fix SC2155
>>>     shellcheck: Fix SC2143
>>>     shellcheck: Fix SC2013
>>>     shellcheck: Fix SC2145
>>>     shellcheck: Fix SC2124
>>>     shellcheck: Fix SC2294
>>>     shellcheck: Fix SC2178
>>>     shellcheck: Fix SC2048
>>>     shellcheck: Suppress various messages
>>
>> I went ahead and pushed a bunch of your patches to the k-u-t master branch
>> now. However, there were also some patches which did not apply cleanly to
>> master anymore, so please rebase the remaining patches and then send them again.
> 
> Hey Thomas,
> 
> Yeah the sc patches were based on top of the big series, so some
> collisions expected. I'll look at rebasing.

Ah, ok, we can also try to get in the big series first ... I just lack 
enough spare time for reviewing currently, so it might take a while :-/

  Thomas
Nicholas Piggin April 17, 2024, 1:40 a.m. UTC | #4
On Tue Apr 16, 2024 at 2:46 PM AEST, Thomas Huth wrote:
> On 16/04/2024 05.26, Nicholas Piggin wrote:
> > On Mon Apr 15, 2024 at 9:59 PM AEST, Thomas Huth wrote:
> >> On 06/04/2024 14.38, Nicholas Piggin wrote:
> >>> Tree here
> >>>
> >>> https://gitlab.com/npiggin/kvm-unit-tests/-/tree/shellcheck
> >>>
> >>> Again on top of the "v8 migration, powerpc improvements" series. I
> >>> don't plan to rebase the other way around since it's a lot of work.
> >>> So this is still in RFC until the other big series gets merged.
> >>>
> >>> Thanks to Andrew for a lot of review. A submitted the likely s390x
> >>> bugs separately ahead of this series, and also disabled one of the
> >>> tests and dropped its fix patch as-per review comments. Hence 3 fewer
> >>> patches. Other than that, since last post:
> >>>
> >>> * Tidied commit messages and added some of Andrew's comments.
> >>> * Removed the "SC2034 unused variable" blanket disable, and just
> >>>     suppressed the config.mak and a couple of other warnings.
> >>> * Blanket disabled "SC2235 Use { ..; } instead of (..)" and dropped
> >>>     the fix for it.
> >>> * Change warning suppression comments as per Andrew's review, also
> >>>     mention in the new unittests doc about the "check =" option not
> >>>     allowing whitespace etc in the name since we don't cope with that.
> >>>
> >>> Thanks,
> >>> Nick
> >>>
> >>> Nicholas Piggin (14):
> >>>     Add initial shellcheck checking
> >>>     shellcheck: Fix SC2223
> >>>     shellcheck: Fix SC2295
> >>>     shellcheck: Fix SC2094
> >>>     shellcheck: Fix SC2006
> >>>     shellcheck: Fix SC2155
> >>>     shellcheck: Fix SC2143
> >>>     shellcheck: Fix SC2013
> >>>     shellcheck: Fix SC2145
> >>>     shellcheck: Fix SC2124
> >>>     shellcheck: Fix SC2294
> >>>     shellcheck: Fix SC2178
> >>>     shellcheck: Fix SC2048
> >>>     shellcheck: Suppress various messages
> >>
> >> I went ahead and pushed a bunch of your patches to the k-u-t master branch
> >> now. However, there were also some patches which did not apply cleanly to
> >> master anymore, so please rebase the remaining patches and then send them again.
> > 
> > Hey Thomas,
> > 
> > Yeah the sc patches were based on top of the big series, so some
> > collisions expected. I'll look at rebasing.
>
> Ah, ok, we can also try to get in the big series first ... I just lack 

They should have come first, but I'd written the multi migration code
before Andrew suggested adding sc, and it looked like hard work to
rebase the other way. I'll try again.

> enough spare time for reviewing currently, so it might take a while :-/

Understandable. You've done heaps of reviewing already so I really
appreciate it.

Thanks,
Nick