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

From: Bart Van Assche
Date: Mon Aug 28 2023 - 14:56:09 EST


On 8/28/23 10:02, Daniel Wagner wrote:
I am against adding code just to make ShellCheck happy.
Hi Daniel,

That's not what my concern is about. My concern is about keeping
the blktests source code maintainable and easy to read. My opinion
is that the ability of bash to pass arguments from caller to callee
implicitly (a) hurts readability, (b) is error prone and (c) hurts
maintainability. This is why I think that this feature should not
be used and hence that disabling SC2119 would be really wrong.

Regarding (a), I think this long e-mail thread is more than enough
evidence that it is not clear what the intention is of the
_nvmet_target_setup calls without arguments - not pass any arguments
or pass the argument list of the caller. Regarding (c): if any
_nvmet_target_setup calls would be added in a function that accepts
arguments, how is _nvmet_target_setup() expected to process arguments
of which it doesn't know how to interpret these?

Hence my proposal to change the _nvmet_target_setup calls with no
arguments into "_nvmet_target_setup --" and also to ignore the double
hyphen argument inside _nvmet_target_setup().

Thanks,

Bart.