2023-08-25 09:29:35

by Daniel Wagner

[permalink] [raw]
Subject: Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

On Fri, Aug 25, 2023 at 12:53:26AM +0000, Shinichiro Kawasaki wrote:
> This works, but a bit ugly. Another idea is to make one of the optional
> arguments mandatory, a positional argument. I think the option --device_type
> worth making mandatory and explicit, like,
>
> _nvmet_target_setup device
> _nvmet_target_setup file

Possible but as I said in the other mail, we have a lot more of default
arguments to functions which I would like to drop too.

> This will make it easier for me to review which test case uses which type.
> (This might be against Sagi's comment, though.)
>
> Daniel, what do you think?

I don't think it is good strategy to change blktests just to make
ShellCheck happy, when we know it is broken. It rather have SC2119
ignored until ShellCheck is fixed.