From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Kuba Kozak <kubax.kozak@intel.com>,
Jacek Piasecki <jacekx.piasecki@intel.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH 1/4] ethdev: add retrieving xstats by group and xstats by name
Date: Mon, 06 Mar 2017 11:17:54 +0100 [thread overview]
Message-ID: <1634037.UG6F3tOjLg@xps13> (raw)
In-Reply-To: <1488545672-5208-2-git-send-email-kubax.kozak@intel.com>
Hi,
2017-03-03 13:54, Kuba Kozak:
> From: Jacek Piasecki <jacekx.piasecki@intel.com>
>
> This patch extends library for retriving xstats by specified groups and
> single xstat by given name.
>
> Signed-off-by: Jacek Piasecki <jacekx.piasecki@intel.com>
> Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>
As you probably know, it is difficult to have a good review
(or a simple review at all).
The most difficult part in such a change is not writing the code,
it is explaining your idea and getting people to agree.
Please show you have thought about the API, seen an issue or a lack,
and propose an idea with a clear target.
In short, we are missing a "why" and "how".
You must also explain what is a group, better than a list of #define.
next prev parent reply other threads:[~2017-03-06 10:17 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-03 12:54 [PATCH 0/4] extend API to retriving xstats by group and xstats by name Kuba Kozak
2017-03-03 12:54 ` [PATCH 1/4] ethdev: add retrieving " Kuba Kozak
2017-03-06 10:17 ` Thomas Monjalon [this message]
2017-03-03 12:54 ` [PATCH 2/4] net/e1000: add grouping of xstats for e1000 driver Kuba Kozak
2017-03-06 2:05 ` Lu, Wenzhuo
2017-03-03 12:54 ` [PATCH 3/4] net/ixgbe: add grouping of xstats for ixgbe driver Kuba Kozak
2017-03-03 12:54 ` [PATCH 4/4] app/proc_info: add support for xstats-name and xstats-group Kuba Kozak
-- strict thread matches above, loose matches on Subject: below --
2017-03-02 16:07 [PATCH 0/4] extend API to retriving xstats by group and xstats by name Kuba Kozak
2017-03-02 16:07 ` [PATCH 1/4] ethdev: add retrieving " Kuba Kozak
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=1634037.UG6F3tOjLg@xps13 \
--to=thomas.monjalon@6wind.com \
--cc=dev@dpdk.org \
--cc=jacekx.piasecki@intel.com \
--cc=kubax.kozak@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.