Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Jerin Jacob <jerinjacobk@gmail.com>]

On Tue, May 26, 2020 at 4:04 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 26/05/2020 12:16, Jerin Jacob:
> > Burakov, Anatoly wrote:
> > > On 25-May-20 7:44 PM, Morten Brørup wrote:
> > > > From: Thomas Monjalon
> > > >> 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.
>
> Let's call these roles "component maintainer" and "tree maintainer".
>
>
> > 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.
>
> Indeed, patchwork is about patch integration status.
> The delegated person is the one responsible for merge, not review.

Yes. That's the problem. Merge will happen after the review.
Who is the owner of the review? It should be
"component maintainer" and it needs to track somewhere to see
the status and know the trend for release.
That make more transparent and know the fate on the patch on early
stage.


>
> > IMO, patch review will speed up or improve if we give the
> > responsibility of the patch to
> > specific maintainer based on the MAINTAINERS file.
>
> I partially disagree about being strict on review responsibility.
> Reviews can and should be done by anyone in the community.
> This is how we scale: avoid bottlenecks.
> Reminder: multiple reviewers are better, and should be the standard.

Yes. No disagreement on that. Anyone free to review. None of the above proposed
the scheme is stopping it.

>
> The component maintainer responsibility is doing some reviews of course,
> but the main responsibility is to make sure reviews are done.
>
>
> > 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 git-send-email option --cc-cmd devtools/get-maintainer.sh
> does a good job at finding code owners.
>
>
> > 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.
>
> Not sure I understood your proposal. You would like one more column
> in patchwork which is automatically filled with code owners?

Yes and the patch's primary _review_ responsibly will be with code owners
and "tree maintainer" apply the patch in fixed time when gets ack from
primary code
owner and if there is no comment from "tree maintainers".


>
>
> > > 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 :)
>
> You can send a new version of the patch with the details fixed,
> publicly readable, reviewable, and ready to be pushed.
>
>
> > > So, while Thomas is free to "fix on apply" at his own desire, i don't
> > > think we have to make this a habit.
>
> Yes, it should be more or less an exception.
>
> Bottom line, it is important to be transparent and predictable,
> while keeping some flexibility.

I agree.

>
>