[bug#61894] [PATCH RFC] Team approval for patches

[Maxim Cournoyer]

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Hello!
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
> [...]
>
>>> “Pacify” in the sense that, by being explicit, we avoid
>>> misunderstandings that could turn into unpleasant experiences.
>>>
>>> Like you I’m glad collaboration is nice and friendly; yet, over the past
>>> few months I’ve experienced misunderstandings that seemingly broke the
>>> consensus-based process that has always prevailed.
>>
>> I'm sorry that you feel that way.  I don't think consensus was willfully
>> broken,
>
> That’s my point: by being explicit about approval, we would avoid such
> misunderstandings.
>
>> and perhaps by studying some actual examples of these occurrences we
>> can better understand what went wrong and how the new suggested policy
>> would have helped or could be modified to help avoid such problems in
>> the future.
>
> I don’t want to rehash past occurrences of this problem.  It boils down
> to: changes where pushed despite consensus evidently not being met, at
> least not in the mind of every involved party.
>
> To some extent, that’s bound to happen due to an increase of the number
> of contributors, scope of the project, and diversity of backgrounds.  By
> making it clear that lack of “LGTM” from another team member equates
> with lack of consensus, we would avoid those misunderstandings.

I agree that firm conventions here would make things smoother.  It's
common that someone offers an incomplete review or just forget the LGTM,
and the author is left to ask if it's OK to push or not or assume it is.

> A good reference on consensus-based decision making is
> <https://www.seedsforchange.org.uk/consensus>.
>
>> It's also worth noting that this consensus-based process has always
>> been implicit; for example, it is not defined/mentioned anywhere in
>> our documentation.  Perhaps it should?
>
> Those who’ve followed the project long enough, such as part of the
> current maintainer collective, are certainly aware of that; it’s also
> spelled out in
> <https://guix.gnu.org/blog/2019/gnu-guix-maintainer-collective-expands/>.
>
> That said, again in the spirit of improving legibility, writing it down
> would be much welcome.

Yes, I've heard 'consensus' for years, and I think I have a good enough
understanding of it, but I think there are subtleties many (including
myself) fail to appreciate that the above link explains well, so I think
there's value in mentioning it somewhere.  I'll send a patch for
everyone to review.

>>> In a way, that’s probably bound to happen as the group grows, and I
>>> think that’s why we must be explicit about what the process is and about
>>> whether one is expressing consent or dissent.
>>>
>>> With so many things happening in Guix (yay!), it’s also easy to overlook
>>> a change and realize when it’s too late.  By having a rule that at least
>>> one other person on the team must approve (consent to) a change, we
>>> reduce that risk.
>>>
>>> Being on a team, then, is a way to express interest on a topic and to be
>>> “in the loop”.
>>
>> That's already what teams can do!
>
> Yes and no.  With the amount of activity going on, it’s easy to overlook
> something.  The explicit synchronization point could mitigate that.
>
>> I'd argue giving them the extra powers that would be conferred to
>> teams in this is not needed/desirable.  Some committer not a regular
>> member of X team may still be confident enough to push a patch sitting
>> on the tracker, and I think they should be able to.
>
> Self-assessment becomes tricky that this scale; I might be confident and
> yet someone will point out a problem (that literally happened to me two
> days ago in <https://issues.guix.gnu.org/62062>).  That’s when review
> really helps.
>
> For “core” work, I insist that explicit approval (and thus peer review)
> is necessary.  I doubt anyone would seriously challenge that.
>
> Now, I agree, as I wrote before, that this may be overkill for “random
> packages”.
>
> Thus we need to find the right balance.
>
> What about team/scope-specific rules?  As in: “Changes covered by teams
> X, Y, and Z need to be explicitly approved by at least one other member
> of the team.”

To me, it seems challenging to partition the code correctly and avoid
overloading the core team with spurious changes, which would slow things
down more (as the core team would be a fraction of the committers
currently enabled to push such changes), but I agree in principle that
it makes sense.

>>> It is not about asserting power or building a hierarchy;
>>> it’s about formalizing existing relations and processes.
>>
>> OK; I think in practice it would amount to that though (building a
>> hierarchy which has some form power).
>
> I disagree: just because power relations are not spelled out doesn’t
> mean they don’t exist.  I don’t know where you’re talking from; one
> thing that to me shed light on these matters is “The Tyranny of
> Structurelessness” (I’m sure I mentioned it before, I certainly did
> during Q&A on this very topic at the Ten Years event; apologies if I
> sound like a broken record!).

As with anything there's probably a middle ground to reach, where
there's some structure, but not too much, that maximizes our collective
happiness.

-- 
Thanks,
Maxim