DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] doc: update contribution guideline for dependent work
@ 2017-11-21 19:59 Ferruh Yigit
  2017-12-11 14:26 ` Mcnamara, John
  2018-01-09 15:44 ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
  0 siblings, 2 replies; 11+ messages in thread
From: Ferruh Yigit @ 2017-11-21 19:59 UTC (permalink / raw)
  To: Thomas Monjalon, John McNamara; +Cc: dev, Ferruh Yigit, techboard

Changing some part of the libraries but not updating all dependent code
cause maintenance problems.

The update in the contribution guide practically suggest updating all
dependent code.

With the counter argument that sometimes it is not possible to know
details of the dependent code and this prevents improvements, an
exception left for the cases dependent code requires special expertise,
for those cases it is possible left dependent code unchanged with the
condition to not break the existing code. The dependent code maintainer
should do the required update later.

For the cases it is possible to update the dependent code but it
requires extra work, it is expected from author of the original patch to
do that extra work and update dependencies.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
This is a process related update, cc'ing tech board for more comment.
Cc: techboard@dpdk.org
---
 doc/guides/contributing/patches.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index 40983c150..7effe3fa5 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -130,6 +130,9 @@ Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines
 * Don't break compilation between commits with forward dependencies in a patchset.
   Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
 
+* If changes effect other parts of the project, update all those parts as well unless updating requires special knowledge.
+  For the cases not all effected code updated, ensure that changes don't break existing code.
+
 * Add tests to the the ``app/test`` unit test framework where possible.
 
 * Add documentation, if relevant, in the form of Doxygen comments or a User Guide in RST format.
-- 
2.13.6

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: update contribution guideline for dependent work
  2017-11-21 19:59 [dpdk-dev] [PATCH] doc: update contribution guideline for dependent work Ferruh Yigit
@ 2017-12-11 14:26 ` Mcnamara, John
  2017-12-12 15:54   ` [dpdk-dev] [dpdk-techboard] " Olivier MATZ
  2018-01-09 15:44 ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
  1 sibling, 1 reply; 11+ messages in thread
From: Mcnamara, John @ 2017-12-11 14:26 UTC (permalink / raw)
  To: Yigit, Ferruh, Thomas Monjalon; +Cc: dev, techboard



> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, November 21, 2017 7:59 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>;
> techboard@dpdk.org
> Subject: [PATCH] doc: update contribution guideline for dependent work
> 
> Changing some part of the libraries but not updating all dependent code
> cause maintenance problems.
> 
> ...
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 


integration testing.
> 
> +* If changes effect other parts of the project, update all those parts as
> well unless updating requires special knowledge.
> +  For the cases not all effected code updated, ensure that changes don't
> break existing code.
> +
>  * Add tests to the the ``app/test`` unit test framework where possible.
> 
>  * Add documentation, if relevant, in the form of Doxygen comments or a
> User Guide in RST format.


Second line would be better as:


For the cases where not all the effected code is updated, the submitter should ensure that changes don't break existing code.


Acked-by: John McNamara <john.mcnamara@intel.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-techboard] [PATCH] doc: update contribution guideline for dependent work
  2017-12-11 14:26 ` Mcnamara, John
@ 2017-12-12 15:54   ` Olivier MATZ
  2017-12-12 18:57     ` Ferruh Yigit
  0 siblings, 1 reply; 11+ messages in thread
From: Olivier MATZ @ 2017-12-12 15:54 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: Yigit, Ferruh, Thomas Monjalon, dev, techboard

Hi,

On Mon, Dec 11, 2017 at 02:26:34PM +0000, Mcnamara, John wrote:
> From: Yigit, Ferruh
> > Changing some part of the libraries but not updating all dependent code
> > cause maintenance problems.
> > 
> > ...
> > 
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > 
> 
> 
> integration testing.
> > 
> > +* If changes effect other parts of the project, update all those parts as
> > well unless updating requires special knowledge.

I feel that "requiring special knowledge" is a bit blury. Shouldn't we add some
examples? Typically, I'm thinking about changes in ethdev that imply updating
the PMDs. Any opinion for this use case?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-techboard] [PATCH] doc: update contribution guideline for dependent work
  2017-12-12 15:54   ` [dpdk-dev] [dpdk-techboard] " Olivier MATZ
@ 2017-12-12 18:57     ` Ferruh Yigit
  2017-12-13  8:55       ` Olivier MATZ
  0 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2017-12-12 18:57 UTC (permalink / raw)
  To: Olivier MATZ, Mcnamara, John; +Cc: Thomas Monjalon, dev, techboard

On 12/12/2017 7:54 AM, Olivier MATZ wrote:
> Hi,
> 
> On Mon, Dec 11, 2017 at 02:26:34PM +0000, Mcnamara, John wrote:
>> From: Yigit, Ferruh
>>> Changing some part of the libraries but not updating all dependent code
>>> cause maintenance problems.
>>>
>>> ...
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>
>>
>>
>> integration testing.
>>>
>>> +* If changes effect other parts of the project, update all those parts as
>>> well unless updating requires special knowledge.
> 
> I feel that "requiring special knowledge" is a bit blury. 

Yes it is, but hard to define where to put the line. My point is if author has
enough knowledge to go and update dependent part, please do so.

> Shouldn't we add some
> examples? Typically, I'm thinking about changes in ethdev that imply updating
> the PMDs. Any opinion for this use case?

Overall many libraries to PMDs fit into this. eal/mbuf/ethdev -> PMD changes.

I think, the dynamic logging update in the other libraries and PMDs should be
done with the original patch, adding dynamic logging to any library doesn't
require library specific information, but agree this is more work.

For new ethdev offload method, I believe it fits into more gray area, it may be
possible to update PMDs to use new offloading method but some part PMDs can be
challenging.

Converting flow director filtering to the rte_flow filtering is something I
believe fair to expect from PMD owner instead of rte_flow author.

> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-techboard] [PATCH] doc: update contribution guideline for dependent work
  2017-12-12 18:57     ` Ferruh Yigit
@ 2017-12-13  8:55       ` Olivier MATZ
  0 siblings, 0 replies; 11+ messages in thread
From: Olivier MATZ @ 2017-12-13  8:55 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Mcnamara, John, Thomas Monjalon, dev, techboard

On Tue, Dec 12, 2017 at 10:57:48AM -0800, Ferruh Yigit wrote:
> On 12/12/2017 7:54 AM, Olivier MATZ wrote:
> > Hi,
> > 
> > On Mon, Dec 11, 2017 at 02:26:34PM +0000, Mcnamara, John wrote:
> >> From: Yigit, Ferruh
> >>> Changing some part of the libraries but not updating all dependent code
> >>> cause maintenance problems.
> >>>
> >>> ...
> >>>
> >>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>
> >>
> >>
> >> integration testing.
> >>>
> >>> +* If changes effect other parts of the project, update all those parts as
> >>> well unless updating requires special knowledge.
> > 
> > I feel that "requiring special knowledge" is a bit blury. 
> 
> Yes it is, but hard to define where to put the line. My point is if author has
> enough knowledge to go and update dependent part, please do so.
> 
> > Shouldn't we add some
> > examples? Typically, I'm thinking about changes in ethdev that imply updating
> > the PMDs. Any opinion for this use case?
> 
> Overall many libraries to PMDs fit into this. eal/mbuf/ethdev -> PMD changes.
> 
> I think, the dynamic logging update in the other libraries and PMDs should be
> done with the original patch, adding dynamic logging to any library doesn't
> require library specific information, but agree this is more work.

Being the author of this patchset, I can give my feeling in this particular
case. That's right adding dynamic logging to all libraries may not require
to know the specifics or the library.

I did these changes to help me while debugging the i40e driver. Once
done, it was worth doing clean EAL upstreamable patches to lay the
foundations for a generic dynamic logging system in DPDK. If I had to do
the work for all the libraries, I would not have done it, because it was
really out of scope of my task.

So the choice was between having nothing, or having something which is
not much used at first, but can be more widely adopted over time.

That said, I understand that the problem with the second approach is to
stay for too long in a situation where the old system is used.

> For new ethdev offload method, I believe it fits into more gray area, it may be
> possible to update PMDs to use new offloading method but some part PMDs can be
> challenging.
> 
> Converting flow director filtering to the rte_flow filtering is something I
> believe fair to expect from PMD owner instead of rte_flow author.

Yes, agree for these 2 examples.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v2] doc: update contribution guideline for dependent work
  2017-11-21 19:59 [dpdk-dev] [PATCH] doc: update contribution guideline for dependent work Ferruh Yigit
  2017-12-11 14:26 ` Mcnamara, John
@ 2018-01-09 15:44 ` Ferruh Yigit
  2018-05-24 16:58   ` Thomas Monjalon
  1 sibling, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2018-01-09 15:44 UTC (permalink / raw)
  To: Ferruh Yigit, John McNamara, Marko Kovacevic; +Cc: dev, techboard

Changing some part of the libraries but not updating all dependent code
cause maintenance problems.

The update in the contribution guide practically suggest updating all
dependent code.

With the counter argument that sometimes it is not possible to know
details of the dependent code and this prevents improvements, an
exception left for the cases dependent code requires special expertise,
for those cases it is possible left dependent code unchanged with the
condition to not break the existing code. The dependent code maintainer
should do the required update later.

For the cases it is possible to update the dependent code but it
requires extra work, it is expected from author of the original patch to
do that extra work and update dependencies.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: John McNamara <john.mcnamara@intel.com>

---
This is a process related update, cc'ing tech board for more comment.
Cc: techboard@dpdk.org

v2: reworded as suggested.
---
 doc/guides/contributing/patches.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index 64408e788..6ed43be04 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -153,6 +153,9 @@ Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines
 * Don't break compilation between commits with forward dependencies in a patchset.
   Each commit should compile on its own to allow for ``git bisect`` and continuous integration testing.
 
+* If changes effect other parts of the project, update all those parts as well unless updating requires special knowledge.
+  For the cases where not all the effected code is updated, the submitter should ensure that changes don't break existing code.
+
 * Add tests to the the ``app/test`` unit test framework where possible.
 
 * Add documentation, if relevant, in the form of Doxygen comments or a User Guide in RST format.
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: update contribution guideline for dependent work
  2018-01-09 15:44 ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
@ 2018-05-24 16:58   ` Thomas Monjalon
  2019-04-05 15:00     ` Ferruh Yigit
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Monjalon @ 2018-05-24 16:58 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, John McNamara, Marko Kovacevic, techboard

09/01/2018 16:44, Ferruh Yigit:
> +* If changes effect other parts of the project, update all those parts as well unless updating requires special knowledge.
> +  For the cases where not all the effected code is updated, the submitter should ensure that changes don't break existing code.

We should review it again in the technical board.
I think we should encourage asking for help to complete a patch
with community's help.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: update contribution guideline for dependent work
  2018-05-24 16:58   ` Thomas Monjalon
@ 2019-04-05 15:00     ` Ferruh Yigit
  2019-04-05 15:00       ` Ferruh Yigit
  2019-04-05 15:02       ` [dpdk-dev] [dpdk-techboard] " Ferruh Yigit
  0 siblings, 2 replies; 11+ messages in thread
From: Ferruh Yigit @ 2019-04-05 15:00 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, John McNamara, Marko Kovacevic, techboard

On 5/24/2018 5:58 PM, Thomas Monjalon wrote:
> 09/01/2018 16:44, Ferruh Yigit:
>> +* If changes effect other parts of the project, update all those parts as well unless updating requires special knowledge.
>> +  For the cases where not all the effected code is updated, the submitter should ensure that changes don't break existing code.
> 
> We should review it again in the technical board.
> I think we should encourage asking for help to complete a patch
> with community's help.

self-nack

This was process update patch, but it seems lack of consensus for now.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: update contribution guideline for dependent work
  2019-04-05 15:00     ` Ferruh Yigit
@ 2019-04-05 15:00       ` Ferruh Yigit
  2019-04-05 15:02       ` [dpdk-dev] [dpdk-techboard] " Ferruh Yigit
  1 sibling, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2019-04-05 15:00 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, John McNamara, Marko Kovacevic, techboard

On 5/24/2018 5:58 PM, Thomas Monjalon wrote:
> 09/01/2018 16:44, Ferruh Yigit:
>> +* If changes effect other parts of the project, update all those parts as well unless updating requires special knowledge.
>> +  For the cases where not all the effected code is updated, the submitter should ensure that changes don't break existing code.
> 
> We should review it again in the technical board.
> I think we should encourage asking for help to complete a patch
> with community's help.

self-nack

This was process update patch, but it seems lack of consensus for now.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-techboard] [PATCH v2] doc: update contribution guideline for dependent work
  2019-04-05 15:00     ` Ferruh Yigit
  2019-04-05 15:00       ` Ferruh Yigit
@ 2019-04-05 15:02       ` Ferruh Yigit
  2019-04-05 15:02         ` Ferruh Yigit
  1 sibling, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2019-04-05 15:02 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, John McNamara, Marko Kovacevic, techboard

On 4/5/2019 4:00 PM, Ferruh Yigit wrote:
> On 5/24/2018 5:58 PM, Thomas Monjalon wrote:
>> 09/01/2018 16:44, Ferruh Yigit:
>>> +* If changes effect other parts of the project, update all those parts as well unless updating requires special knowledge.
>>> +  For the cases where not all the effected code is updated, the submitter should ensure that changes don't break existing code.
>>
>> We should review it again in the technical board.
>> I think we should encourage asking for help to complete a patch
>> with community's help.
> 
> self-nack
> 
> This was process update patch, but it seems lack of consensus for now.
> 

for reference, mentioned patch:
https://patches.dpdk.org/patch/33250/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-techboard] [PATCH v2] doc: update contribution guideline for dependent work
  2019-04-05 15:02       ` [dpdk-dev] [dpdk-techboard] " Ferruh Yigit
@ 2019-04-05 15:02         ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2019-04-05 15:02 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, John McNamara, Marko Kovacevic, techboard

On 4/5/2019 4:00 PM, Ferruh Yigit wrote:
> On 5/24/2018 5:58 PM, Thomas Monjalon wrote:
>> 09/01/2018 16:44, Ferruh Yigit:
>>> +* If changes effect other parts of the project, update all those parts as well unless updating requires special knowledge.
>>> +  For the cases where not all the effected code is updated, the submitter should ensure that changes don't break existing code.
>>
>> We should review it again in the technical board.
>> I think we should encourage asking for help to complete a patch
>> with community's help.
> 
> self-nack
> 
> This was process update patch, but it seems lack of consensus for now.
> 

for reference, mentioned patch:
https://patches.dpdk.org/patch/33250/


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-04-05 15:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-21 19:59 [dpdk-dev] [PATCH] doc: update contribution guideline for dependent work Ferruh Yigit
2017-12-11 14:26 ` Mcnamara, John
2017-12-12 15:54   ` [dpdk-dev] [dpdk-techboard] " Olivier MATZ
2017-12-12 18:57     ` Ferruh Yigit
2017-12-13  8:55       ` Olivier MATZ
2018-01-09 15:44 ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
2018-05-24 16:58   ` Thomas Monjalon
2019-04-05 15:00     ` Ferruh Yigit
2019-04-05 15:00       ` Ferruh Yigit
2019-04-05 15:02       ` [dpdk-dev] [dpdk-techboard] " Ferruh Yigit
2019-04-05 15:02         ` Ferruh Yigit

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).