Re: [dpdk-dev] Consider improving the DPDK contribution processes

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 25-May-20 10:34 AM, Morten Brørup wrote:
> Dear DPDK Techboard,
> 
> I am writing this to raise awareness about the environment for contributing to DPDK, as I feel that it could be improved. This is not a personal thing - I have thick skin - but a general observation. I urge the DPDK Techboard to spend some time to focus on the process, and not only on the technology.
> 
> Contributing to DPDK is not easy for infrequent contributors:
> 
> 1. Infrequent contributors are limited by not being deeply familiar with the coding style and the commit style, so their style is not always 100 % spot on.
> 2. Infrequent contributors are limited by not having built trust by the maintainers and frequent contributors, and thus their contributions undergo more detailed reviews and get more negative (or: perceived negative) feedback, where trusted contributors are given more slack. (In theory, every contribution should be treated equal, but in reality it makes sense allocating fewer resources to review contributions from developers with a proven track record.)
> 3. Infrequent contributors may not be deeply familiar with the development/contribution tools. E.g. how to use git the "DPDK way".
> 
> Additionally, when contributing to old DPDK code, checkpatch complains about coding style violations stemming from the existing old code. This also raises the barrier and decreases the motivation to contribute - a contributor getting negative feedback about something he didn't even do.
> 
> 
> Here are a couple of anonymous examples from the mailing list:
> 
> An infrequent contributor got minor coding style suggestions to a patch, although the coding style was similar to that of a closely related function in the same library, but not perfectly matching the official coding style. I think we could be more lax about coding style, except if the coding style directly violates automatic coding style validation tools.
> 

A lot of that could simply be fixed by codifying our Coding Style into a 
.clang-format file, and make this process (semi-)automatic. A lot of 
IDE's/editors now have either built-in support for clang-format, or have 
plugins enabling said support.

I've investigated this in the past and found that our coding style 
guidelines are very specific in some places, and neither clang-format 
nor other options have that kind of detailed control over source code 
formatting. The only other option would be to adjust our coding style to 
fit the options available in clang-format.

IMO this would cut down a lot on complaints about mixing indents, wrong 
alignment, (lack of) newlines before function name, etc.

> Another infrequent contributor got patch style feedback about a small patch, suggesting to split it up into three patches. The official contributing guide says that small changes should be kept together in the same patch. The patch in question could be considered three bug fixes, so splitting it up might be appropriate, or it could be considered fixing three variations of the same bug, so keeping it together as one would be appropriate. I think we could be more lax about patch style, except if the patches are completely incomprehensible.

In my experience, this has been up to individual maintainers. I 
generally split up my patches not to follow a guideline, but to make 
reviewing code easier for others. Sometimes that means splitting up the 
patch, sometimes that means keeping everything together, and sometimes 
either one is OK so i don't insist on either.

But perhaps it would be a good idea to clarify some of these guidelines.

Then again, we should also remember that coding guidelines are there to 
help us all, and to avoid wasting maintainers' time: David/Thomas/Ferruh 
manually fixing every patch before applying just because we are "lax" in 
reviews doesn't really scale. So, it's really a double edged sword IMO.

> 
> It is not all bad, though! I have more than once seen excellent feedback to a suggested patch, where an expert reviewer took the time and effort to explain - in an educational and welcoming tone - what was wrong about the patch and the contributor's assumptions.
> 
> 
> In summary, I think that DPDK has grown to a point where we hopefully will see more contributions from outsiders and newcomers, and we need to adapt to it, so that they get a positive experience from contributing, and keep coming back with more.
> 
> Improving the process should be an ongoing discussion. Here are two areas for discussion:
> 
> 1. Keep expanding the contributor documentation. It is already far better than for most projects, but there is always room for improvement. Assume that the reader is intelligent, but has limited information in advance. E.g. Patchwork and Bugzilla are barely mentioned, and it is only described how to use them when submitting a patch.
> 
> 2. When giving feedback on the mailing list, consider how it may be received, weighing the balance between accepting imperfection and educating the contributor.

I would add a third area: the process itself is arcane and inaccessible. 
The current consensus among the community seems to be that IRC + mailing 
list are "the most accessible" because "everyone has email" and "getting 
on IRC is easy".

However, the truth is, they aren't "accessible", they are low tech, and 
thus *in*accessible to those who aren't veteran command-line Linux 
coders. No one uses IRC any more except OSS projects (so a new 
contributor isn't likely to have an IRC set up unless they're already a 
habitual contributor to other projects), and sending patches over an 
email as opposed to a well-integrated web interface workflow is so alien 
to most people that it definitely does discourage new contributions.

I understand the advantages of mailing lists (vendor independence, 
universal compatibility, etc.), but after doing reviews in Github/Gitlab 
for a while (we use those internally), going through DPDK mailing list 
and reviewing code over email fills me with existential dread, as the 
process feels so manual and 19th century to me.

> 
> 
> DPDK is a great project, and the contribution process described above sounds a lot worse than it actually is. But I just can't get enough rainbows and unicorns! :-)
> 
> 
> Med venlig hilsen / kind regards
> 
> Morten Brørup
> CTO
> 
> 
> SmartShare Systems A/S
> Tonsbakken 16-18
> DK-2740 Skovlunde
> Denmark
> 
> Office      +45 70 20 00 93
> Direct      +45 89 93 50 22
> Mobile     +45 25 40 82 12
> 
> mb@smartsharesystems.com
> www.smartsharesystems.com
> 


-- 
Thanks,
Anatoly