Re: [PATCH 2/6] app/testpmd: register driver specific commands

[Thomas Monjalon <thomas@monjalon.net>]

24/05/2022 19:44, David Marchand:
> On Tue, May 24, 2022 at 7:21 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > 23/05/2022 09:10, David Marchand:
> > > +# Driver specific sources include some testpmd headers.
> >
> > Suggested reword:
> >         Driver-specific commands are located in driver directories.
> 
> At the moment, it's only about testpmd commands.
> Maybe in the future we can have other specific code in those driver sources.
> That's why I preferred a generic term.

It is a comment in the context of testpmd here.

[...]
> > > +#define TESTPMD_ADD_DRIVER_COMMANDS(c) \
> >
> > Why not TESTPMD_ADD_COMMANDS?
> 
> I forgot to align both the function and macro.
> Please note though that for now, the registered commands through this
> API get displayed in the "Driver specific" usage section.
> So maybe the API should state it is about driver specific commands.
> 
> WDYT?

You're right, it should be driver-specific commands.

> > [...]
> > > --- a/drivers/meson.build
> > > +++ b/drivers/meson.build
> > > +        if testpmd_sources.length() != 0
> > > +            testpmd_drivers_sources += testpmd_sources
> > > +            testpmd_drivers_deps += dependency_name
> > > +        endif
> >
> > Are you sure the check is required?
> > What happens if adding an empty string?
> 
> We don't want to push a driver dependency to testpmd if there is nothing to add.

Oh yes, OK.