On Mon, Jun 13, 2022 at 02:08:30PM +0100, Carsten Haitzler wrote:
[...]
If the condition checking gets complex, seems to me it is reasonable to use a static function (or a macro?) to encapsulate the logics.
Well normally my rule i s - if it gets re-used then do it, otherwise it just involves more indirection to follow. :) But regardless of that, given some other things you ask for that kind of makes this discussion moot as it requires much bigger wholesale changes to the test infra which will make these patches a lot more work. I'll get to that later in mails.
Your mentioned rule makes sense to me.
But one catch... it really should be is_non_hidden_exe_shell_script() as it's checking that it's not a hidden file AND is a shell script. Or do I keep the hidden file test outside of the function in the if? If we're nit picking then I need to know exactly what you want here as your suggested name is actually incorrect.
I personally prefer to use the condition:
if (is_exe_shell_script() && ent->d_name[0] != '.') do_something...
The reason is the function is_exe_shell_script() is more common and we use it easily in wider scope.
As above - will probably have to redo a lot of the test infra involving the shell tests to handle some of your other requests, but if we don't go that way, I have got where you want to go and I can do this.
To be honest, I am not sure if this patch is related with refactoring test infrastructure or not. You could reconsider when you spin for next patch set (as you said, might refactor test infra).
In case you still want to keep this patch as it is, it would be fine for me and you could add my reviewed tag:
Reviewed-by: Leo Yan leo.yan@linaro.org
Thanks, Leo