Re: [dpdk-dev] [PATCH v6 4/5] examples/kni: add log msgs to show and clear stats

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 24 Oct 2018 17:56:50 -0300
Dan Gora <dg@adax.com> wrote:

> On Wed, Oct 24, 2018 at 5:46 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > > +     pid = getpid();
> > > +     RTE_LOG(INFO, APP, "========================\n");
> > > +     RTE_LOG(INFO, APP, "KNI Running\n");
> > > +     RTE_LOG(INFO, APP, "kill -SIGUSR1 %d\n", pid);
> > > +     RTE_LOG(INFO, APP, "    Show KNI Statistics.\n");
> > > +     RTE_LOG(INFO, APP, "kill -SIGUSR2 %d\n", pid);
> > > +     RTE_LOG(INFO, APP, "    Zero KNI Statistics.\n");
> > > +     RTE_LOG(INFO, APP, "========================\n");
> > > +     fflush(stdout);
> > > +  
> >
> > Thanks for updating the example to cover the new code.
> >
> > This seems like an overly wordy message which should really be in the documentation
> > not a billboard in the code.
> >
> > In my opinion, having verbose messages is unhelpful since it just clutters
> > the experience.  
> 
> Sigh...
> 
> This is version 6 of this patch.  You could have said something about
> it at any point in the last two and a half months that I have been
> struggling to get this merged.
> 
> These "features" were never documented at all, so you would have no
> idea they existed unless you read the code.
> 
> The point of this patch is that you can just copy and paste the
> commands directly from the screen.  This saves you from having to type
> 'ps -ef|grep kni', cut the PID, type 'kill -SIGUSR1', then paste the
> PID.  How is that easier that what I have done?
> 
> And it's not a billboard, it's 7 lines.  Have you actually tried it?
> 
> The amount of nitpicking on these patches has been just incredible..
> People get entire subsystems merged with 1/10th the hassle that I've
> been given to add one stupid function.  It's extremely frustrating.
> I've totally given up on trying to get my other KNI patches merged..
> It's just not worth it..
> 
> dan

I look at patches as they show up and don't want to overwhelm people
with a long laundry list of items. Just a case of call them as I see them.
Often a developer is focused on "does my feature work" and misses how
the new feature is not used by most people.

Remember when working on projects that the unstated policy is that all
code should look the same. Anything you introduce should look like everything
around it. Yes, this limits taste and individual freedom, but if you want to
change things then doing it in new code is not the way to do it.

The patch can go in as is. There is no reason for a message to block that.
Just trying to see what can be improved.

Don't get disheartened, 6 versions of a patch is nothing bad.
Sometimes it takes 20 or more until agreement occurs.