From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DDF3FA04EF; Mon, 25 May 2020 13:12:55 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B3A991D8F1; Mon, 25 May 2020 13:12:55 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 4FF331D8F0; Mon, 25 May 2020 13:12:53 +0200 (CEST) IronPort-SDR: EVwg3MyzZddoShfBbKVCWrgNv+D6bnxtN0jDoHLjadlHpYj7CRDiE+yvwypOwpfF4IjUpnzHBt nfiP4zu/SXlg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 May 2020 04:12:52 -0700 IronPort-SDR: fbpTp45VWctYKFV/vRCLj/uByblR1842AiIX+GtwUGakpTxMjFJMtUhKv+qYG1Zs7MjphqOlQR btlCEuBV5liQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,433,1583222400"; d="scan'208";a="468024579" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.212.230.177]) ([10.212.230.177]) by fmsmga006.fm.intel.com with ESMTP; 25 May 2020 04:12:50 -0700 To: =?UTF-8?Q?Morten_Br=c3=b8rup?= , techboard@dpdk.org Cc: dev@dpdk.org, "Jim St. Leger" References: <98CBD80474FA8B44BF855DF32C47DC35C60FEA@smartserver.smartshare.dk> From: "Burakov, Anatoly" Message-ID: <6d59a42a-915b-47fc-60e6-94a4600d4bff@intel.com> Date: Mon, 25 May 2020 12:12:49 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35C60FEA@smartserver.smartshare.dk> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] Consider improving the DPDK contribution processes X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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