Subject: [alpine-devel] RFC: Soft commit rights for established maintainers

From: William Pitcock <[email-masked]>
Date: Tue, 21 Mar 2017 03:21:11 -0500

Hello,

As we all know, new contributors to Alpine who wish to become Alpine
Developers go through a process to gain commit rights, meanwhile
Maintainers have to find a Developer to commit their work by proxy.

As the project has grown, multiple people have introduced new channels
for maintainers to work in the distribution, such as submitting
patches through GitHub.  These contribution channels have yielded
great results for Alpine, resulting in many new maintainers stepping
forward, some of which have over time gone through the full process
and gained commit rights.  To be quite clear: those initiatives have
been largely successful.

However, working through the GitHub and Patchwork queues looking for
packages to commit, I notice that a majority of the commits are just
generated by abump, which got me thinking...

Is it really worth developer time to go through a bunch of abump
patches?  Is it worth making Alpine users wait for someone to go
through and commit their changes?  In my opinion, most of the time,
*new* packages are where the review is needed.  If the change
ultimately is just a new SHA512 hash and $version change, then for
established maintainers, I think it is alright to just accept the
change into the distribution.  And if that is the case, then why not
allow them to push the change themselves?

What I propose is quite simple -- we add a git hook to check incoming
pushes with the following checks:

1. Is the committer a proper dev?  If so, allow the commit through.

2. Is the commit only changing packages (and thusly not configs stored
in the aports tree)?  If not, reject the push.

3. Are the changed APKBUILDs in question maintained by the person who
committed the patch (or they have additional privilege, maybe we can
add a Co-Maintainer field?).  If not, reject the push.

4. Are the changed APKBUILDs on a whitelist for the maintainer?  If
not, reject the push.

5. If all the above checks pass (2 through 4), allow the push.

This would allow us to extend some "soft" commit rights to new
prospective developers, which in turn gives *positive reinforcement*
that they are on track to becoming a developer.  It also has the
benefit of reducing the size of the aports patch queues and allowing
us to concentrate review efforts on people who need those efforts the
most.

There is some minor risk that folks who have the soft commit rights
extended to them may from time to time mess up and cause a builder to
stall.  However, in these cases we can simply revert the push or
correct it.  After all, that is what we would be doing with a broken
GitHub submission.  Beyond that, quite honestly, we have all done it
before anyway *and* if the maintainer breaks something, it gives them
the possibility of fixing it themselves before it causes any major
impact.  So overall, I would say that it is a very minor risk.

William

---
Unsubscribe:  [email-masked]
Help:         [email-masked]
---

---

From: Natanael Copa <[email-masked]>
Date: Tue, 21 Mar 2017 09:56:58 +0100

On Tue, 21 Mar 2017 03:21:11 -0500
William Pitcock <[email-masked]> wrote:

> patches? 

No, but we need verify that maintainer does not push ABI breaking
change without trigger rebuild of the needed packages.

> Is it worth making Alpine users wait for someone to go

I agree that this is the direction we want go.

A thing that might be simpler to implement ans a step in that direction
is a github hook that will check if the commiter is the same as the
maintainer, and if it is, add a label "maintainer" or similar to to the
pull request. That way we can fast-track maintainer updates.

we could have similar hooks for simple version bumps, add a label
"bump" or similar, regardless if it is maintainer or not.

Finally, if both the "bump" and "maintainer" label is set, then we
could merge of those.

---