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
m...@smartsharesystems.com
www.smartsharesystems.com
--
Thanks,
Anatoly
