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:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---

On Tue, 21 Mar 2017 03:21:11 -0500
William Pitcock <nenolod@dereferenced.org> wrote:

> 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? 

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
> 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.

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.
 
> 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:  alpine-devel+unsubscribe@lists.alpinelinux.org
> Help:         alpine-devel+help@lists.alpinelinux.org
> ---
> 



---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---