Re: [dpdk-dev] [PATCH v1] app/test-regex: add RegEx test application

[Thomas Monjalon <thomas@monjalon.net>]

27/07/2020 07:12, Ori Kam:
> From: Jerin Jacob <jerinjacobk@gmail.com>
> > On Mon, Jul 27, 2020 at 1:28 AM Ori Kam <orika@mellanox.com> wrote:
> > > --- /dev/null
> > > +++ b/app/test-regex/hello_world.rof2
> > > @@ -0,0 +1,45 @@
> > > +#
> > > +# rof_version: 2
> > > +#
> > > +# date:20200210_164643
> > > +#
> > > +# rxp_compiler:5.7.18007
> > 
> > Please don't check-in vendor/driver specific file formats in the main
> > repository.
> > See below.
> 
> I fully agree with you that in normal cases such files should not be part of
> the main repository, but this case is different from the regard that this test
> can't be run without vendor specific files. I expect that each vendor will
> add its own file for this repo.

I don't think it's "different".
As an open source project, we prefer dealing only with source files.
We must provide the tools to generate required binaries.

[...]
> > > +Running the Tool
> > > +----------------
> > > +
> > > +The tool has a number of command line options. Here is the sample
> > > command line:

In docs, it's better to have each sentence on its own line.

> > > +
> > > +.. code-block:: console

If you end previous line with "::" instead of ":" then you can drop
the code-block line.

> > > +
> > > +   ./build/app/testregex -w 83:00.0 -- --rules app/test-regex/hello_world.rof2
> > > --data app/test-regex/input.txt --job 100

Don't write too much long lines in verbatim blocks.

> > 
> > Instead of giving the binary rule format, Primary option could to be
> > compile the rule by the application itself.
> > If the driver does not have such capability then the application can
> > look for binary rule file in such case please don't host
> > binary rules in dpdk.org.
> 
> Like I said above, in Mellanox case the rule must be precompiled,
> (I think the same goes for number of other vendors) and it doesn’t look
> complete code, if we will just give the code,
> and the user will have to download files from other places.
> 
> I think for this specific example and since the file is small we should allow it.

No

> What do you think? 

We should provide all the tools.
If the tool is external, it must become a requirement to run the app.