Message-ID: <20241129032150.13f8aa35@foz.lan>
Date: Fri, 29 Nov 2024 03:21:50 +0100
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Simona Vetter <simona.vetter@...ll.ch>
Cc: Jani Nikula <jani.nikula@...el.com>, 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 22:52:38 +0100
Simona Vetter <simona.vetter@...ll.ch> escreveu:

> On Thu, 28 Nov 2024 at 22:27, Jani Nikula <jani.nikula@...el.com> wrote:
> > On Thu, 28 Nov 2024, Mauro Carvalho Chehab <mchehab+huawei@...nel.org> wrote:  
> > > 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.  
> >
> > While I understand the hesitation, I don't think it's fair towards
> > potential future collaborators to distrust them based on a bad actor
> > from nearly 20 years ago.  
> 
> Yeah this sounds a lot more like a CoC issue (which of course could
> result in a very swift removal of commit rights and suspend from all
> access to gitlab and mailing lists). Aside from reference the CoC
> we've left these things out of scope of the commit rights processes
> and merge criteria.
>
> My key takeaway over the last decade more of maintainering is that
> assuming that people want to do the right thing and building a process
> optimized for that works really well. And then handle the toxic people
> entirely separately through solid conduct enforcement.
>

Community has evolved and CoC may help, but it is still it is dozen
times more painful to remove grants than to not add rights for people
that aren't ready yet to become committers. 

The migration to the new model is already complex enough with experienced
people having troubles with the new CI engine and new process.
With just to people testing the new process, basically every time we 
committed something, we discovered one or more issues with the CI that 
would end denying merges and cause frustration and more work to
maintainers.

> > >> 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  
> >
> > It's not two reviewers for us either; it's typically author+reviewer and
> > either author or reviewer commits. Two sets of eyeballs in total.
> >  
> > > beginning for that, but maintainers may revert/rebase the tree in
> > > case they don't agree with changes.  
> >
> > Not sure if you really mean it, but saying it like that doesn't really
> > breed trust, IMO. Sure, there have been patches merged to i915 that I
> > didn't "agree" with. But bad enough to warrant a revert? Very few and
> > far between, and always for clear and concrete reasons rather than
> > anything subjective.
> >
> > Side note, we don't do rebases in the development branches.  
> 
> Yeah, if I don't forget anything I remember a grand total of three
> rebases by maintainers, and this over 8 years or so of doing this:
> 
> - Someone pushed their entire development tree by accident. We
> obviously had to back that out and improved the tooling to catch these
> better.
> - Someone who pushed an entire pile of work (I think 30 patches or so)
> that missed the merge window into -fixes for a late -rc1.
> - Someone who lost trust with upstream maintainers because they
> refused to listen for way too long to engineering direction and
> consensus. The last big push of development work was backed out again.
> 
> There might have been some other things, but I think those were more
> maintainers screwing things up than committers pushing stuff, and on
> trees that are handled with the more classic group maintainer model.
> 
> It's really an extremely rare event that we rebase out patches.

Rebases should be rare, and we do avoid doing that, but it depends
on what happens and how the merged tree is tested. We hope that
the workflow we're implementing with CI testing everything will
prevent them, but we need to run it for a few kernel cycles to
be sure that what is there is good enough.

Regards,
Mauro

Powered by blists - more mailing lists