From: "Varghese, Vipin" <vipin.varghese@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: "Pattan, Reshma" <reshma.pattan@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>,
"Mcnamara, John" <john.mcnamara@intel.com>,
"Byrne, Stephen1" <stephen1.byrne@intel.com>,
"Patel, Amol" <amol.patel@intel.com>
Subject: Re: [dpdk-dev] [PATCH v7 0/9] app/proc-info: improve debug of proc-info tool
Date: Thu, 27 Dec 2018 02:46:52 +0000 [thread overview]
Message-ID: <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D2DF4B6@BGSMSX101.gar.corp.intel.com> (raw)
In-Reply-To: <1797411.vFYqA0X0Ny@xps>
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.
> 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.
next prev parent reply other threads:[~2018-12-27 2:46 UTC|newest]
Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-22 14:15 [dpdk-dev] [PATCH v5 1/9] app/procinfo: add usage for new debug Vipin Varghese
2018-11-22 14:15 ` [dpdk-dev] [PATCH v5 2/9] app/procinfo: add compare for new options Vipin Varghese
2018-11-22 14:15 ` [dpdk-dev] [PATCH v5 3/9] app/procinfo: add prototype for debug instances Vipin Varghese
2018-11-22 14:15 ` [dpdk-dev] [PATCH v5 4/9] app/procinfo: add support for show port Vipin Varghese
2018-11-22 14:15 ` [dpdk-dev] [PATCH v5 5/9] app/procinfo: add support for show tm Vipin Varghese
2018-11-22 14:15 ` [dpdk-dev] [PATCH v5 6/9] app/procinfo: add support for show crypto Vipin Varghese
2018-11-22 14:15 ` [dpdk-dev] [PATCH v5 7/9] app/procinfo: add support for debug ring Vipin Varghese
2018-11-22 14:15 ` [dpdk-dev] [PATCH v5 8/9] app/procinfo: add support for show mempool Vipin Varghese
2018-11-22 14:15 ` [dpdk-dev] [PATCH v5 9/9] doc/procinfo: add information for debug options Vipin Varghese
2018-11-22 18:08 ` Mcnamara, John
2018-11-23 4:58 ` Varghese, Vipin
2018-12-03 5:49 ` [dpdk-dev] [PATCH v6 0/9] app/proc-info: improve debug of proc-info tool Vipin Varghese
2018-12-03 5:49 ` [dpdk-dev] [PATCH v6 1/9] app/procinfo: add usage for new debug Vipin Varghese
2018-12-12 14:14 ` Pattan, Reshma
2018-12-12 15:06 ` Varghese, Vipin
2018-12-13 5:08 ` [dpdk-dev] [PATCH v7 0/9] app/proc-info: improve debug of proc-info tool Vipin Varghese
2018-12-13 5:08 ` [dpdk-dev] [PATCH v7 1/9] app/procinfo: add usage for new debug Vipin Varghese
2018-12-13 5:08 ` [dpdk-dev] [PATCH v7 2/9] app/procinfo: add compare for new options Vipin Varghese
2018-12-13 5:08 ` [dpdk-dev] [PATCH v7 3/9] app/procinfo: add prototype for debug instances Vipin Varghese
2018-12-13 5:08 ` [dpdk-dev] [PATCH v7 4/9] app/procinfo: add support for show port Vipin Varghese
2018-12-13 5:08 ` [dpdk-dev] [PATCH v7 5/9] app/procinfo: add support for show tm Vipin Varghese
2018-12-13 5:08 ` [dpdk-dev] [PATCH v7 6/9] app/procinfo: add support for show crypto Vipin Varghese
2018-12-13 5:08 ` [dpdk-dev] [PATCH v7 7/9] app/procinfo: add support for debug ring Vipin Varghese
2018-12-13 5:08 ` [dpdk-dev] [PATCH v7 8/9] app/procinfo: add support for show iter mempool Vipin Varghese
2018-12-13 5:08 ` [dpdk-dev] [PATCH v7 9/9] doc/procinfo: add information for debug options Vipin Varghese
2018-12-13 14:42 ` [dpdk-dev] [PATCH v7 0/9] app/proc-info: improve debug of proc-info tool Pattan, Reshma
2018-12-13 16:21 ` Varghese, Vipin
2018-12-22 21:02 ` Thomas Monjalon
2018-12-26 5:21 ` Varghese, Vipin
2018-12-26 21:33 ` Thomas Monjalon
2018-12-27 2:46 ` Varghese, Vipin [this message]
2018-12-27 9:32 ` Thomas Monjalon
2018-12-27 10:45 ` Varghese, Vipin
2019-01-07 15:38 ` [dpdk-dev] [PATCH v8 0/7] app/proc-info: enhance " Vipin Varghese
2019-01-07 15:38 ` [dpdk-dev] [PATCH v8 1/7] app/procinfo: prepare for new debug functions Vipin Varghese
2019-01-09 23:45 ` Thomas Monjalon
2019-01-10 3:08 ` Varghese, Vipin
2019-01-10 8:39 ` Thomas Monjalon
2019-01-10 8:53 ` Varghese, Vipin
2019-01-10 9:09 ` Thomas Monjalon
2019-01-10 21:36 ` [dpdk-dev] [PATCH v9 0/6] app/proc-info: enhance debug of proc-info tool Vipin Varghese
2019-01-10 21:36 ` [dpdk-dev] [PATCH v9 1/6] app/procinfo: add support for show port Vipin Varghese
2019-01-10 17:40 ` Thomas Monjalon
2019-01-11 3:57 ` Varghese, Vipin
2019-01-11 10:22 ` Thomas Monjalon
2019-01-10 21:36 ` [dpdk-dev] [PATCH v9 2/6] app/procinfo: add support for show tm Vipin Varghese
2019-01-10 21:36 ` [dpdk-dev] [PATCH v9 3/6] app/procinfo: add support for show crypto Vipin Varghese
2019-01-10 21:36 ` [dpdk-dev] [PATCH v9 4/6] app/procinfo: add support for debug ring Vipin Varghese
2019-01-10 21:36 ` [dpdk-dev] [PATCH v9 5/6] app/procinfo: add support for show mempool Vipin Varghese
2019-01-10 21:36 ` [dpdk-dev] [PATCH v9 6/6] app/procinfo: add support for iter mempool Vipin Varghese
2019-01-11 14:46 ` [dpdk-dev] [PATCH v9 0/6] app/proc-info: enhance debug of proc-info tool Thomas Monjalon
2019-01-14 0:07 ` Varghese, Vipin
2019-01-07 15:38 ` [dpdk-dev] [PATCH v8 2/7] app/procinfo: add support for show port Vipin Varghese
2019-01-07 15:38 ` [dpdk-dev] [PATCH v8 3/7] app/procinfo: add support for show tm Vipin Varghese
2019-01-07 15:38 ` [dpdk-dev] [PATCH v8 4/7] app/procinfo: add support for show crypto Vipin Varghese
2019-01-07 15:38 ` [dpdk-dev] [PATCH v8 5/7] app/procinfo: add support for debug ring Vipin Varghese
2019-01-07 15:38 ` [dpdk-dev] [PATCH v8 6/7] app/procinfo: add support for show mempool Vipin Varghese
2019-01-07 15:38 ` [dpdk-dev] [PATCH v8 7/7] app/procinfo: add support for iter mempool Vipin Varghese
2018-12-03 5:49 ` [dpdk-dev] [PATCH v6 2/9] app/procinfo: add compare for new options Vipin Varghese
2018-12-03 5:49 ` [dpdk-dev] [PATCH v6 3/9] app/procinfo: add prototype for debug instances Vipin Varghese
2018-12-03 5:49 ` [dpdk-dev] [PATCH v6 4/9] app/procinfo: add support for show port Vipin Varghese
2018-12-03 5:49 ` [dpdk-dev] [PATCH v6 5/9] app/procinfo: add support for show tm Vipin Varghese
2018-12-03 5:49 ` [dpdk-dev] [PATCH v6 6/9] app/procinfo: add support for show crypto Vipin Varghese
2018-12-03 5:49 ` [dpdk-dev] [PATCH v6 7/9] app/procinfo: add support for debug ring Vipin Varghese
2018-12-03 5:49 ` [dpdk-dev] [PATCH v6 8/9] app/procinfo: add support for show iter mempool Vipin Varghese
2018-12-12 16:10 ` Pattan, Reshma
2018-12-13 2:26 ` Varghese, Vipin
2018-12-03 5:50 ` [dpdk-dev] [PATCH v6 9/9] doc/procinfo: add information for debug options Vipin Varghese
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=4C9E0AB70F954A408CC4ADDBF0F8FA7D4D2DF4B6@BGSMSX101.gar.corp.intel.com \
--to=vipin.varghese@intel.com \
--cc=amol.patel@intel.com \
--cc=dev@dpdk.org \
--cc=john.mcnamara@intel.com \
--cc=konstantin.ananyev@intel.com \
--cc=reshma.pattan@intel.com \
--cc=stephen1.byrne@intel.com \
--cc=stephen@networkplumber.org \
--cc=thomas@monjalon.net \
/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;
as well as URLs for NNTP newsgroup(s).