Hello Thomas,

On Friday 06 October 2017 05:04 AM, Thomas Monjalon wrote:
Signed-off-by: Shreyansh Jain <shreyansh.j...@nxp.com>
Signed-off-by: Hemant Agrawal <hemant.agra...@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.

Reply via email to