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

From: Bart Van Assche
Date: Fri Aug 25 2023 - 09:46:17 EST


On 8/25/23 00:34, Shinichiro Kawasaki wrote:
Recently, you actively cleans up tests/nvme/* (which is great!), and introduced
argument parsers in test/nvme/rc. The first one is _nvme_connect_subsys, and the
second one is this _nvme_target_setup. It looks for me this is a bash coding
style change in blktests, from "don't use optional arguments often" to "use
optional arguments aggressively". If we apply this change, we should suppress
SC2119. If we keep the old coding style, we should keep on enabling SC2119. What
I see here is the style difference between you and Bart.

Now I'm tempted to disable SC2119, and to go with the new coding style...

If I have any misunderstanding, or if anyone has more comments on this, please
let me know.

I don't like the "new style". What is so hard about typing "$@" to pass all function
arguments to _nvmet_target_setup()? Leaving out "$@" makes it much harder than
necessary to figure out the intent of the code author - not passing any arguments
or passing all caller arguments implicitly.

Bart.