Message-ID: <20241128194758.7d2e7656@foz.lan>
Date: Thu, 28 Nov 2024 19:47:58 +0100
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Jani Nikula <jani.nikula@...el.com>
Cc: Simona Vetter <simona.vetter@...ll.ch>, Laurent Pinchart
 <laurent.pinchart@...asonboard.com>, linux-media@...r.kernel.org, Jonathan
 Corbet <corbet@....net>, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, workflows@...r.kernel.org, Hans Verkuil
 <hverkuil@...ll.nl>
Subject: Re: [PATCH] docs: media: document media multi-committers rules and
 process

Em Thu, 28 Nov 2024 13:24:04 +0200
Jani Nikula <jani.nikula@...el.com> escreveu:

> On Wed, 27 Nov 2024, Simona Vetter <simona.vetter@...ll.ch> wrote:
> > Jumping in the middle here with some clarifications.
> >
> > On Wed, 27 Nov 2024 at 12:19, Laurent Pinchart <laurent.pinchart@...asonboard.com> wrote:  
> >> On Wed, Nov 27, 2024 at 10:39:48AM +0100, Mauro Carvalho Chehab wrote:  
> >> > It is somewhat similar to drm-intel and drm-xe, where reviews are part
> >> > of the acceptance criteria to become committers.  
> >>
> >> Those are corporate trees, so it's easier to set such rules.  
> >
> > Imo it's the other way round, because it's corporate you need stricter
> > rules and spell them all out clearly - managers just love to apply
> > pressure on their engineers too much otherwise "because it's our own
> > tree". Totally forgetting that it's still part of the overall upstream,
> > and that they don't own upstream.  
> 
> The required commits and reviews to become a committer may sound
> somewhat artificial and arbitrary, but it's a sort of compromise. The
> goal is to have a relatively low bar for entry, while ensuring the
> committers have just enough experience to judge whether a patch passes
> merge criteria (more on that below). It's also the same for everyone,
> and something to strive for.

We used to have a low bar for entry on our past multi-committers
model (back in 2005-2007). It was a disaster, as one of the
committer did very evil things. He was blocked, but that didn't
prevent some of us to be threatened with physical violence - and 
some people even reported death threats.

So, let's start slow to ensure that things like that won't ever
happen again.

> Frankly, I'm not fond of the invite-only model. You need to be careful
> to not lose transparency.

The intent is to be as transparent as possible without violating regulations
like GPDR.

> It can be scary to have a lot of committers. You put a lot of trust in
> them. But at the same time, you do monitor what's going on, and can
> revert commits - and commit rights, if needed.

The scariest part is to receive threats like what happened in the past.
Some were publicly documented; others happened via private talks and
e-mails.

> > So that's why the corporate trees are stricter than drm-misc, but the
> > goals are still exactly the same:
> >
> > - peer review is required in a tit-for-tat market, but not more.
> >
> > - committers push their own stuff, that's all. Senior committers often
> >   also push other people's work, like for smaller work they just reviewed
> >   or of people they mentor, but it's not required at all.  
> 
> I think it's also important to define merge criteria. A set of rules by
> which a committer can commit. And it's not really about technical
> checkboxes. For example, in drm it really boils down to two things: at
> least two people have been involved, and there are no open issues.

That's the same criteria we're aiming for. We'll start without
two people reviewing, as there won't be enough committers at the
beginning for that, but maintainers may revert/rebase the tree in
case they don't agree with changes.

> (And have those people recorded in git trailers with Sob/Rb/Ab, with
> tooling to ensure that's the case. There are very few commis in
> drm-misc/drm-intel without either 2xSob, Sob+Rb, or Sob+Ab.)

We added a CI engine to check this and other issues. If CI fails,
commit will be automatically be blocked.

> > - maintainership duties, like sending around pr, making sure patches dont
> >   get lost and things like that, is separate from commit rights. In my
> >   opinion, if you tie commit rights to maintainership you're doing
> >   something else than drm and I'd more call it a group maintainership
> >   model, not a commit rights model for landing patches.  
> 
> Agreed. Personally, I like the committer/maintainer model, because it's
> a low barrier to entry, with limited responsibilities. Not everyone
> wants to become even a committer, and the more loaded it becomes, even
> less so. Yet the committers help maintainers immensely, and you grow a
> pool of people who can become maintainers.

Currently, for most of the drivers, the number of committers per driver
is equal to the number of maintainers for the same driver.

So, on this stage, we're aiming on get maintainers commit rights,
starting with the ones that are long time contributors and regularly
participate at the media summits.

Once the "slow start" phase finishes, we can review the process and
start thinking on getting more developers and committers.

> > Anyway just figured I'll clarify what we do over at drm. I haven't looked
> > at all the details of this proposal here and the already lengthy
> > discussion, plus it's really not on me to chime in since I'm not involved.  
> 
> To be honest, IMO the length is one of the shortcomings of the
> proposal. Lots of up front process, which will inevitably have to be
> ironed out as you gain experience. You just won't be able to figure it
> all out in advance.

Agreed.

> All that said, I commend all efforts to move towards shared
> maintainership models, whether it's group maintainership or
> committer/maintainer model or something in between. Just offering my
> views here, which you're obviously free to completely ignore to your
> benefit or detriment. ;)

Thank you for you valuable feedback!

Best regards,
Mauro

Powered by blists - more mailing lists