From: Leo Yan <leo.yan@linaro.org>
To: Carsten Haitzler <carsten.haitzler@foss.arm.com>
Cc: linux-kernel@vger.kernel.org, coresight@lists.linaro.org,
suzuki.poulose@arm.com, mathieu.poirier@linaro.org,
mike.leach@linaro.org, linux-perf-users@vger.kernel.org,
acme@kernel.org
Subject: Re: [PATCH 2/3] perf test: Shell - only run .sh shell files to skip other files
Date: Wed, 15 Jun 2022 17:14:06 +0800 [thread overview]
Message-ID: <20220615091406.GB121786@leoy-ThinkPad-X240s> (raw)
In-Reply-To: <ae8bdc2c-a444-74c1-70d5-c97d27b22ec5@foss.arm.com>
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
next prev parent reply other threads:[~2022-06-15 9:15 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-09 12:28 [PATCH 1/3] perf test: Shell - Limit to only run executable scripts in tests carsten.haitzler
2022-03-09 12:28 ` [PATCH 2/3] perf test: Shell - only run .sh shell files to skip other files carsten.haitzler
2022-04-10 2:28 ` Leo Yan
2022-04-21 16:21 ` Carsten Haitzler
2022-05-26 10:14 ` Leo Yan
2022-06-13 13:08 ` Carsten Haitzler
2022-06-15 9:14 ` Leo Yan [this message]
2022-03-09 12:28 ` [PATCH 3/3] perf test: Add coresight tests to guage quality of data generated carsten.haitzler
2022-04-10 8:30 ` Leo Yan
2022-04-21 17:38 ` Carsten Haitzler
2022-05-26 8:20 ` Leo Yan
2022-05-26 16:08 ` Leo Yan
2022-06-13 14:15 ` Carsten Haitzler
2022-05-30 16:27 ` Mathieu Poirier
2022-05-30 16:47 ` Mathieu Poirier
2022-06-13 12:53 ` Carsten Haitzler
2022-06-13 13:00 ` Carsten Haitzler
2022-04-10 1:24 ` [PATCH 1/3] perf test: Shell - Limit to only run executable scripts in tests Leo Yan
2022-04-11 19:08 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220615091406.GB121786@leoy-ThinkPad-X240s \
--to=leo.yan@linaro.org \
--cc=acme@kernel.org \
--cc=carsten.haitzler@foss.arm.com \
--cc=coresight@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
--cc=mike.leach@linaro.org \
--cc=suzuki.poulose@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox