From: Shreyansh Jain <shreyansh.jain@nxp.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: Hemant Agrawal <hemant.agrawal@nxp.com>, <dev@dpdk.org>,
<ferruh.yigit@intel.com>
Subject: Re: [dpdk-dev] [PATCH] bus/fslmc: update MC to 10.3.x
Date: Fri, 6 Oct 2017 18:41:35 +0530 [thread overview]
Message-ID: <c02fd1ba-8f64-25f6-e3f7-9423be140f68@nxp.com> (raw)
In-Reply-To: <4808052.LEHOq8oruM@xps>
Hello Thomas,
On Friday 06 October 2017 05:04 AM, Thomas Monjalon wrote:
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>
> This is a huge patch without any comment!
> It will be hard to find bugs in it when debugging in future.
> And it will be also pointless to reference it in future fixes.
> From a quality point of view, it is bad.
Well, this patch is actually changing the lowest glue code for DPAA2. I
did split into multiple patches (component wise) but it didn't make much
sense as they were still un-reviewable. I merged them back again.
Ironically, this series was actually to make code easier to read -
improving overall glue code quality. Improving comments and variable
naming, removing large macros - with minimal functional impact.
>
> As it is touching only to dpaa2 stuff, it is accepted.
> I do not want to be the one digging in it.
Fair enough. I am not expecting that someone will review. It is internal
stuff to DPAA2 and not something which can be reviewed without context.
> Please avoid such patch in future.
This is somewhere I have doubt.
The core model of DPAA2/DPAA1 drivers is hardware interfacing code which
is large in size. This layer tends to change when newer internal
versions are released or some update is made (bug fixes).
We don't send changes out very frequently - that is why they gather over
a longer period. Keeping them in original state (multiple patches) only
ends up spoiling DPDK commit history.
Being a single commit, I think bisects would be easier in the sense that
a bisect would either contain or not contain these internal changes.
But, I will definitely take care to make them as commented/documented as
much as possible in future.
(Though, I do take care of splitting any patch which touches the DPDK
API layer into logical splits as that is reflected in commit history.)
>
> Applied
>
Thank a lot.
prev parent reply other threads:[~2017-10-06 12:59 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-16 10:35 [dpdk-dev] [PATCH] NXP DPAA2 bus driver fw upgrade Hemant Agrawal
2017-09-16 10:35 ` [dpdk-dev] [PATCH] bus/fslmc: update MC to 10.3.x Hemant Agrawal
2017-10-05 23:34 ` Thomas Monjalon
2017-10-06 13:11 ` Shreyansh Jain [this message]
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=c02fd1ba-8f64-25f6-e3f7-9423be140f68@nxp.com \
--to=shreyansh.jain@nxp.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=hemant.agrawal@nxp.com \
--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).