Re: [dpdk-dev] [PATCH v7 0/9] app/proc-info: improve debug of proc-info tool

[Thomas Monjalon <thomas@monjalon.net>]

27/12/2018 03:46, Varghese, Vipin:
> snipped
> > > > > > Small nits
> > > > > > 9th patch in this set is doc. So above info need to be corrected.
> > > > > > if you are addressing my earlier comment of separating out
> > > > > > mempool element iteration changes in to separate new patch 9/10
> > > > > > .Please keep my ack in next version
> > > > >
> > > > > Thanks for pointing this out, Like updated in email and chat I am
> > > > > not
> > > > planning to split it. Hence no version 8.
> > > >
> > > > So, no ack and no merge?
> > > >
> > > > Looking at the first patches + doc patch, the split is not meaningful.
> > > > You should merge doc and option parsing in the related patches.
> > > > For instance, parsing and doc of "tm" option should be in the "tm" patch.
> > >
> > > I did not follow you request. Are you stating, for each functionality I should
> > be updating document rather than 1 document update after adding the new
> > functions? If former is true I am not able to find such reasoning stated in
> > guideline or documentation or from the maintainer.
> > 
> > Yes, you should update the doc while adding a new feature.
> Ok, I will comply to your requirement even though it is not in 'guideline, documentation or from maintainer'. Humbly requesting to update documentation and guideline suggesting the same. This will also help others to submit patches according the new guideline. Once reflected it will be justified for sending a v8.

Vipin, please read the doc carefully:
	http://git.dpdk.org/dpdk/commit/?id=9e0e4a00df775

> > But most importantly, there is no reason to do a patch adding some empty
> > functions and filling them later.
> Following are the reasons for using stub function from v1 onwards till v7
> 1. Without the dummy function there are compiler warnings for unused variables.
> 2. It is logical to have stub functions for the new parse option being added in one go.
> 
> These are based on the suggestion from the maintainer.
> 
> > And please consider the option parsing is part of the feature.
> As mentioned above please find the reasoning stated for patches from v1 to v7.

You keep thinking that parsing should be introduced separately.
I keep saying it is part of the feature.