Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon <tho...@monjalon.net>
> 
> 27/07/2020 07:12, Ori Kam:
> > From: Jerin Jacob <jerinjac...@gmail.com>
> > > On Mon, Jul 27, 2020 at 1:28 AM Ori Kam <or...@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.
> 
This "tool" is private and large, so I can't share it. I will remove this file.

> [...]
> > > > +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.
> 
Will change.

> > > > +
> > > > +.. code-block:: console
> 
> If you end previous line with "::" instead of ":" then you can drop
> the code-block line.
> 
Do I still need to keep the same indentation?

> > > > +
> > > > +   ./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.
> 
Will see how to shorten it.

> > >
> > > 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
> 
Will remove this file.

> > What do you think?
> 
> We should provide all the tools.
> If the tool is external, it must become a requirement to run the app.
>
I will modify the doc to set this file as a requirement.
 
> 

Thanks,
Ori

Reply via email to