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 9F4A8A04A4; Tue, 26 May 2020 12:16:33 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D837B1D66C; Tue, 26 May 2020 12:16:32 +0200 (CEST) Received: from mail-io1-f65.google.com (mail-io1-f65.google.com [209.85.166.65]) by dpdk.org (Postfix) with ESMTP id 3590E1D668; Tue, 26 May 2020 12:16:32 +0200 (CEST) Received: by mail-io1-f65.google.com with SMTP id p20so7902455iop.11; Tue, 26 May 2020 03:16:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=kICBqpukf2El8hDi365h+YgC+/KDqsJ7+d2+jA2kdqY=; b=bvYlf8ZxkD0Ovf33yDbsjgqu241Sisifk1mLLQZsestNtElt5eGXE538UQPBkTPeom K4WQARHKYil3B3xCb95Vs6w34gxpgrWWODT9OLO33HDqB1+3Gt4MbnQXA3SZogjNuwOl 8FHCizE232dJULrqXdaX1cpeyjzCDIjwWtRMN221Z4sl+g29NykXUJFQwzUCZGX2l+6l 4e6eQCYHH1EbERyRATdni4TOzgkmmUbOILNDw0XL8Msk6Px5ekDh4Xu0W2KG/2ef7dI6 zMk6taJHTH4+zuVyy/1Sq5pGdJ3lESyMceYu7wgXKm+B2TtV6M3YOqKnHEk4x4yMWUXt KBjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=kICBqpukf2El8hDi365h+YgC+/KDqsJ7+d2+jA2kdqY=; b=Hlah8feTURSGXc0boiw1/i+0rDcafqBxlASgyGdynrYlZWnZ62v4DD6b282+Dxrg+g xAimvYyPoYVOgei89SMBZje5PfXucoOUAqbZDYSLvPW3/uGOk7Tsc52Lg4NJfprQilWz 9Sz+buiuoVjOxM6/agPpOrkiv81/h8VyELMt0E+oCYC7lrhkwGQM3+DbrsFIscadhF1r PozGAa51+Ej8Oto5jcUW27K9KwIhlsFCCQye79LjX5gtGkISH259KcDUNGfUr9Kskiyj l/v2Y8PDxvmNwCMuXXjnHXxxQ8xWNDEsJp2rAhrDxz8TYccTuiFPrudYNqQMqlOukXQP Fo5w== X-Gm-Message-State: AOAM5310lGFoyP9l8URDHbPQzIxWP7OJvtZAz5MWCIoiZLhQndNTkL1W 4udHf/CdlYKChoQBXaSeB1M/BE6uvNalDM3Gezg= X-Google-Smtp-Source: ABdhPJyjZ79608A9evkioqRFirTVVQGr7yUHboRZbHDIyS/Jgpa80ywQE+TkZ05vSBz1vhI50rpNvST7cyDqbBxAMdM= X-Received: by 2002:a6b:1543:: with SMTP id 64mr16918009iov.123.1590488191054; Tue, 26 May 2020 03:16:31 -0700 (PDT) MIME-Version: 1.0 References: <98CBD80474FA8B44BF855DF32C47DC35C60FEA@smartserver.smartshare.dk> <6512da71-09a0-3357-27b1-58939597bcf1@redhat.com> <1d1c7a90-934b-3db4-b7d6-308a0ebb7ee4@intel.com> <11959277.FkLDZFFinP@thomas> <98CBD80474FA8B44BF855DF32C47DC35C60FF5@smartserver.smartshare.dk> In-Reply-To: From: Jerin Jacob Date: Tue, 26 May 2020 15:46:14 +0530 Message-ID: To: "Burakov, Anatoly" Cc: =?UTF-8?Q?Morten_Br=C3=B8rup?= , Thomas Monjalon , Maxime Coquelin , dpdk-dev , techboard@dpdk.org, "Jim St. Leger" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution 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 Tue, May 26, 2020 at 3:13 PM Burakov, Anatoly wrote: > > On 25-May-20 7:44 PM, Morten Br=C3=B8rup wrote: > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon > >> Sent: Monday, May 25, 2020 6:29 PM > >> > >> 25/05/2020 18:09, Burakov, Anatoly: > >>> On 25-May-20 5:04 PM, Maxime Coquelin wrote: > >>>> On 5/25/20 5:59 PM, Burakov, Anatoly wrote: > >>>>> On 25-May-20 4:52 PM, Maxime Coquelin wrote: > >>>>>> On 5/25/20 5:35 PM, Jerin Jacob wrote: > >>>>>>> On May 25, 2020 Thomas Monjalon wrote: > >>>>>>>> My concern about clarity is the history of the discussion. > >>>>>>>> When we post a new versions in GitHub, it's very hard to keep > >> track > >>>>>>>> of the history. > >>>>>>>> As a maintainer, I need to see the history to understand what > >> happened, > >>>>>>>> what we are waiting for, and what should be merged. > >>>>>>> > >>>>>>> IMO, The complete history is available per pull request URL. > >>>>>>> I think, Github also email notification mechanism those to > >> prefer to see > >>>>>>> comments in the email too. > >>>>>>> > >>>>>>> In addition to that, Bugzilla, patchwork, CI stuff all > >> integrated into > >>>>>>> one place. > >>>>>>> I am quite impressed with vscode community collaboration. > >>>>>>> https://github.com/Microsoft/vscode/pulls > >>>>>> > >>>>>> Out of curiosity, just checked the git history and I'm not that > >>>>>> impressed. For example last commit on the master branch: > >>>>>> > >>>>>> > >> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7= 4 > >> 46b3915d6148 > >>>>>> > >>>>>> > >>>>>> Commit title: " Fix #98530 " > >>>>>> Commit message empty, no explanation on what the patch is doing. > >>>>>> > >>>>>> Then, let's check the the issue it is pointed to: > >>>>>> https://github.com/microsoft/vscode/issues/98530 > >>>>>> > >>>>>> Issue is created 15 minutes before the patch is being merged. All > >> that > >>>>>> done by the same contributor, without any review. > >>>>>> > >>>>> > >>>>> Just because they do it wrong doesn't mean we can't do it right :) > >> This > >>>>> says more about Microsoft's lack of process around VSCode than it > >> does > >>>>> about Github the tool. > >>>>> > >>>> > >>>> True. I was just pointing out that is not the kind of process I > >> would > >>>> personally want to adopt. > >>>> > >>> > >>> You won't find disagreement here, but this "process" is not due to > >> the > >>> tool. You can just as well allow Thomas to merge stuff without any > >>> review because he has commit rights, no Github needed - and you would > >> be > >>> faced with the same problem. > >>> > >>> So, i don't think Jerin was suggesting that we degrade our > >> merge/commit > >>> rules. Rather, the point was that (whatever you think of VSCode's > >>> review/merge process) there are a lot of pull requests and there is > >>> healthy community collaboration. I'm not saying we don't have that, > >> > >> Yes, recent survey said the process was fine: > >> http://mails.dpdk.org/archives/announce/2019-June/000268.html > >> > >> > >>> obviously, but i have a suspicion that we'll get more of it if we > >> lower > >>> the barrier for entry (not the barrier for merge!). I think there is > >> a > >>> way to lower the secondary skill level needed to contribute to DPDK > >>> without lowering coding/merge standards with it. > > > > That is exactly what I am asking for: Lowering the barrier and increasi= ng the feeling of success for newcomers. (The barrier for merge is probably= fine; I'll leave that discussion to the maintainers.) > > > >> > >> About the barrier for entry, maybe it is not obvious because I don't > >> communicate a lot about it, but please be aware that I (and other > >> maintainers I think) are doing a lot of changes in newcomer patches > >> to avoid asking them knowing the whole process from the beginning. > >> Then frequent contributors get educated on the way. > > > > Great! I wish that every developer would think and behave this way. > > Part of the problem is, there are two different maintainers here: > maintainers like myself, who maintain a certain area of the code, and > maintainers like Thomas, who has *commit rights* and maintains the > entire tree. Yes. I had a similar thought when I said about the "fine-grained" maintainership/ownership. The patchwork does not reflect the fine-grained owner of this patch series. IMO, patch review will speed up or improve if we give the responsibility of the patch to specific maintainer based on the MAINTAINERS file. Github picks up the default owner as CODEOWNERS(The PRIMARY one responsible for the file or section of the code) https://github.com/zephyrproject-rtos/zephyr/blob/master/CODEOWNERS. The policy for merge permission( Can CODEOWNERS merge the patch) will be specific to the project. IMO, This scheme would enable CODEOWNERS are more responsible for the code review and we have need system where query what is pending the CODEOWERS. This http://patches.dpdk.org/project/dpdk/ list does not depict the CODEOWERS in DPDK. > > And therein lies the problem: Thomas (David, etc.) doesn't look at every > area of the code, he relies on us to do it. However, *he* is doing the > committing, and fixing up patches, etc. - so, i can't really say things > like, "hey, your indentation's wrong here, but Thomas will fix it on > apply" because that's me pushing more work onto Thomas, something i > don't think i have the moral right to do :) > > So, while Thomas is free to "fix on apply" at his own desire, i don't > think we have to make this a habit. > > > > >> > >> I think the only real barrier we have is to sign the patch > >> with a real name and send an email to right list. > >> The ask for SoB real name is probably what started this thread > >> in Morten's mind. And the SoB requirement will *never* change. > > > > The incorrect Signed-off-by might be the only hard barrier (which we ca= nnot avoid). But that did not trigger me. > > > > I was raising the discussion to bring attention to soft barriers for co= ntributors. What triggered me was the request to split the patch into multi= ple patches; a kind of feedback I have seen before. For an experienced git = user, this is probably very easy, but for a git newbie (like myself), it ba= sically means starting all over and trying to figure out the right set of g= it commands to do this, which can be perceived as a difficult task requirin= g a lot of effort. > > > > Perhaps we could supplement the Contributor Guidelines with a set of co= okbooks for different steps in the contribution process, so reviewers can b= e refer newcomers to the relevant of these as part of the feedback. Just li= ke any professional customer support team has a set of canned answers ready= for common customer issues. (Please note: I am not suggesting adding an AI= /ML chat bot reviewer to the mailing list!) > > That's a great idea, although it's arguably slightly out of scope for > DPDK. Then again, we do have a "fixline" instructions, so why not have a > "git reset HEAD^ && git add -p" in there while we're at it :) > > > > > The amount of Contributor Guideline documentation is also a balance... = it must be long enough to contain the relevant information to get going, bu= t short enough for newcomers to bother reading it. > > > > > -- > Thanks, > Anatoly