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

[Ludovic Courtès]

Hello Guix!

The project has been steadily gaining new contributors, which is great,
and I think we need to adjust our processes accordingly.

Currently teams are described mostly as pools of people who can mentor
contributors in a particular area and who can review patches in that
area.  My proposal is to give teams formal approval power over changes
to code in their area.

This is sorta happening already, but informally: if a non-committer
sends a patch, someone from the team eventually “approves” it by pushing
it.  Within a team, the situation is different: people usually discuss
changes, and the submitter (also committer) eventually pushes them;
sometimes, the submitter pushes changes without getting approval (or
feedback) from others on the team.

With the proposed policy, members of a team would also have to review
and approve each other’s work.  Formal approval means getting an
explicit “LGTM” (or similar) from at least one other team member.

This is similar to the review thresholds found on GitLab & co., where
project admins can specify a minimum number of approvals required before
a change is marked as ready.  I think it avoids the unavoidable
misunderstandings that can arise in a growing group and help pacify
day-to-day collaboration.

Below is a suggested change.

What do people think?

Ludo’.

diff --git a/doc/contributing.texi b/doc/contributing.texi
index c436bc4a31..d8ca6802c4 100644
--- a/doc/contributing.texi
+++ b/doc/contributing.texi
@@ -1486,7 +1486,7 @@ reply to incoming bugs and patches, which contains the 
bug number.
 @anchor{Notifying Teams}
 @cindex teams
 The @file{etc/teams.scm} script may be used to notify all those who
-may be interested in your patch of its existence (@pxref{Teams}).
+may be interested in your patch and may approve it (@pxref{Teams}).
 Use @command{etc/teams.scm list-teams} to display all the teams,
 decide which team(s) your patch relates to, and use
 @command{etc/teams.scm cc} to output various @command{git send-email}
@@ -1557,6 +1557,9 @@ these changes are necessary.
 @subsection Teams
 @cindex teams
 
+The project is structured as @dfn{teams}, which play two related roles:
+mentoring people who contribute code in their area of expertise, and
+reviewing and approving changes to that code.
 There are several teams mentoring different parts of the Guix source
 code.  To list all those teams, you can run from a Guix checkout:
 
@@ -1840,8 +1843,14 @@ Patches}).  It also allows patches to be picked up and 
tested by the
 quality assurance tooling; the result of that testing eventually shows
 up on the dashboard at
 @indicateurl{https://qa.guix.gnu.org/issue/@var{ISSUE_NUMBER}}, where
-@var{ISSUE_NUMBER} is the number assigned by the issue tracker.  Leave
-time for a review, without committing anything (@pxref{Submitting
+@var{ISSUE_NUMBER} is the number assigned by the issue tracker.
+
+When your patch falls under the area of expertise of a team
+(@pxref{Teams}), you need the explicit approval of at least one team
+member before committing (another team member if you are on the team).
+
+In other cases,
+leave time for a review, without committing anything (@pxref{Submitting
 Patches}).  If you didn’t receive any reply after one week (two weeks
 for more significant changes), and if you're confident, it's OK to
 commit.